feat(code_review): Handle GitHub re-run check requests #104455
Conversation
❌ 13 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
| @@ -0,0 +1 @@ | |||
| # Error prediction module for Seer integration | |||
There was a problem hiding this comment.
maybe we should call it code_review that is closer to the external name of the feature?
Change the logic to actually handle the webhook payload, extract the info that is relevant to send to Seer and then send it (to an updated endpoint) See getsentry/seer#4173 for more details.
96ef5a7 to
0a1bd89
Compare
|
bugbot review |
| status = e.__class__.__name__ | ||
| metrics.incr(f"{PREFIX}.outcome", tags={"status": f"error_{status}"}) | ||
| if self.request.retries >= MAX_RETRIES - 1: | ||
| logger.exception("%s.error", PREFIX, extra=context) |
There was a problem hiding this comment.
additionally here -- since we use sentry, we'll have the full stacktrace in the issue -- do we really have to log the status?
There was a problem hiding this comment.
Could you please rephrase this?
status is reported in the finally: metrics.incr(f"{PREFIX}.outcome", tags={"status": status})
There was a problem hiding this comment.
Does it still apply? I've addressed some of your feedback.
GabeVillalobos
left a comment
There was a problem hiding this comment.
Changes on the integration side seem reasonable to me. Thanks for running this by us!
JoshFerge
left a comment
There was a problem hiding this comment.
seems okay. i feel that the error handling is very over-defensive and will result in a lot of noisy telemetry / makes the code more complicated that it needs to be. And more of a stylistic thing but I also feel that a lot of the comments in this PR feel unnecessary as they just restate what the code is doing.
I will be addressing both. I changed AGENTS.md to reduce the unnecessary comments. |
We previously had something about comments in our AGENTS.md and it didn't seem to help -- i think the models are tuned to be extremely verbose |
|
|
||
| # This needs to match the value defined in the Seer API: | ||
| # https://github.com/getsentry/seer/blob/main/src/seer/routes/codegen.py | ||
| SEER_PR_REVIEW_RERUN_PATH = "/v1/automation/codegen/pr-review/rerun" |
There was a problem hiding this comment.
|
@seer review |
Thank you @JoshFerge for always pressing back and finding bugs! |
suejung-sentry
left a comment
There was a problem hiding this comment.
This is great! Added a comment on options to update the "code review eligibility" check logic to match the current state of things as we get ready for GA
| """ | ||
| Determine if the GitHub check_run event should be handled. | ||
| """ | ||
| if not features.has("organizations:code-review-beta", organization=organization): |
There was a problem hiding this comment.
Since we plan to ship this PR this week, let's use the logic in this check can_use_prevent_ai_features
PLUS
If the org has seat-based-seer-enabled flag on (representing that they've bought the new seat-based seer plan), we should ALSO CHECK whether their repo is enabled using something like below:
repo_settings = (
RepositorySettings.objects.select_related("repository")
.filter(
repository__external_id=external_repo_id,
repository__organization_id=sentry_org_id,
repository__provider=provider, // "integrations:github"
repository__status=ObjectStatus.ACTIVE,
enabled_code_review=True,
)
.first()
)
This is the logic I hope to get pulled into an all-in-one helper with the overwatch migration project.
If we prefer instead to tackle this complexity separately and just expose the check-run functionality for the code review beta cohort to start, we can update to check the value of these 4 flags code-review-beta, gen-ai-features, hide_ai_features, enable_pr_review_test_generation (basically what is in can_use_prevent_ai_features but with the additional check that the org is in the code-review-beta cohort).
There was a problem hiding this comment.
@suejung-sentry has graciously followed-up on this here: https://github.com/getsentry/sentry/pull/105236/changes
For code reviews, we create a GitHub check. In some cases, the customer may request the check to be re-run (e.g. the task failed or timed out).
This handles the GitHub re-run event by scheduling a task which calls Seer with the original run check ID to re-schedule it (it requires https://github.com/getsentry/seer/pull/4173).
Fixes CW-18.