Conversation
Split repository lookup, provider resolution, start SHA selection, and SCM lifecycle fetch handling into focused helpers. This keeps the main fetch_commits loop linear and easier to reason about without changing behavior. Made-with: Cursor
github-actions
bot
added
the
Scope: Backend
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicate "fetch_commits.start" log entry per invocation
- Removed the duplicate logger.info('fetch_commits.start', ...) call at line 332, keeping only the new enhanced version at line 353 with richer context.
Or push these changes by commenting:
@cursor push 0fb9386369
Preview (0fb9386369)
diff --git a/src/sentry/tasks/commits.py b/src/sentry/tasks/commits.py
--- a/src/sentry/tasks/commits.py
+++ b/src/sentry/tasks/commits.py
@@ -329,7 +329,6 @@
commit_list: list[dict[str, Any]] = []
release = Release.objects.get(id=release_id)
- logger.info("fetch_commits.start", extra={"organization_slug": release.organization.slug})
set_tag("organization.slug", release.organization.slug)
# TODO: Need a better way to error handle no user_id. We need the SDK to be able to call this without user context
# to autoassociate commits to releasesYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 54e4b99. Configure here.
Guard span status updates when no active span is available so the original exception handling path can send notifications and record lifecycle state. Rename the second start-phase log event to avoid duplicate fetch_commits.start entries per invocation. Made-with: Cursor
|
|
||
|
|
||
| def get_repo_and_provider_for_ref( | ||
| def get_repo_for_ref( |
There was a problem hiding this comment.
Decoupling this function into two.
This first one will only return the repo.
| return repo | ||
|
|
||
|
|
||
| def get_provider_for_repo( |
There was a problem hiding this comment.
This second one only focuses on getting the provider.
| return provider, is_integration_repo_provider, provider_key | ||
|
|
||
|
|
||
| def get_start_sha_for_ref( |
There was a problem hiding this comment.
This function is extracted from the main loop.
| return None | ||
|
|
||
|
|
||
| def fetch_commits_for_ref_with_lifecycle( |
There was a problem hiding this comment.
This is the main logic which happened within the for loop.
The else part of the block is not ported:
sentry/src/sentry/tasks/commits.py
Lines 305 to 317 in 4e67edb
| } | ||
| logger.info("fetch_commits.config", extra=extra) | ||
|
|
||
| for ref in refs: |
There was a problem hiding this comment.
Most of the changes in this PR are about making this for loop easier to read by moving a lot of the code blocks into their own functions.
| logger.info("fetch_commits.config", extra=extra) | ||
|
|
||
| for ref in refs: | ||
| resolved = get_repo_and_provider_for_ref(release=release, ref=ref, user_id=user_id) |
There was a problem hiding this comment.
We're splitting this function into two to keep their logic simpler (even if we added a few more lines here).
|
|
||
| end_sha = ref["commit"] | ||
| extra = extra | {"repository": repo.name, "end_sha": end_sha, "start_sha": start_sha} | ||
| logger.info("fetch_commits.loop.start", extra=extra) |
armenzg
changed the title
1 participant
Refactor the release commit-fetching task by extracting focused helpers for repository lookup, provider resolution, start SHA selection, and lifecycle-wrapped commit fetching.
This keeps
fetch_commitseasier to follow and reason about as a linear orchestration flow while preserving the existing behavior and error handling paths.I considered only extracting the lifecycle block, but splitting repo lookup and provider resolution as separate helpers makes each step explicit and aligns with the recent follow-up refactors requested during review.
Additional context: logging now uses shared per-loop context and still records commit counts without changing commit association semantics.
Made with Cursor