feat(git): harden Bitbucket Cloud connector reliability - #1843
Merged
Conversation
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughChangesBitbucket Cloud ingestion was redesigned around a shared API client, deterministic repository buckets, incremental repository state, expanded pull request and resource streams, snapshot-aware dbt models, and updated integration and reliability tests. Bitbucket Cloud ingestion
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SourceBitbucketCloud
participant BitbucketClient
participant RepositoryCatalog
participant BitbucketStream
participant BitbucketIncrementalStream
SourceBitbucketCloud->>BitbucketClient: construct client and check workspaces
SourceBitbucketCloud->>RepositoryCatalog: construct repository catalog
BitbucketStream->>RepositoryCatalog: select repositories by bucket
BitbucketStream->>BitbucketClient: fetch repository resources or commit ranges
BitbucketIncrementalStream->>BitbucketIncrementalStream: emit records and commit repository state
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)src/ingestion/connectors/git/bitbucket-cloud/dbt/schema.ymlTraceback (most recent call last): Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
One failing repository no longer aborts the remaining repositories in its bucket: each repository is processed under a quarantine that logs the failure, keeps its incremental state untouched, and surfaces an error at the end of the sync. Stream read loops are unified behind a repository_records template method; branch listings are memoized on the repository catalog so commits, file changes, and reachability share one consistent snapshot per sync. Client hardening: commit range pagination uses pagelen=100, pagination refuses next-URLs outside the configured API base, and invalid JSON responses raise a clear error instead of an opaque traceback. Invalid bitbucket_start_date values now fail with an actionable message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
aleksdotbar
force-pushed
the
rewrite/bitbucket-cloud-improvements
branch
from
July 22, 2026 11:41
8ba0a12 to
21920e7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/ingestion/connectors/git/bitbucket-cloud/dbt/schema.yml (1)
24-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFreshness SLA now applies to optional resources that may legitimately be empty.
The source-level
freshness(error_after 72h) covers every listed table. Optional resources likepipelines,pipeline_steps,pipeline_step_test_reports,deployments,environments,tags, andissuesare commonly unused in a given repo/workspace, so their bronze tables stay empty or stale and will trip freshness failures with no underlying issue. Consider overridingfreshness: nullper-table for these optional streams (dbt supports table-level freshness overrides) to avoid alert noise.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ingestion/connectors/git/bitbucket-cloud/dbt/schema.yml` around lines 24 - 32, Override the table-level freshness to null for the optional resources pipelines, pipeline_steps, pipeline_step_test_reports, deployments, environments, tags, and issues in the schema definitions. Leave issue_comments and issue_changes unchanged unless they are also explicitly optional, preserving the source-level 72-hour freshness SLA for required streams.src/ingestion/connectors/git/bitbucket-cloud/tests/test_branches.py (1)
28-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReachability tests miss the "moved branch" and failure-fallback paths.
Coverage only exercises
"added"(new branch) and"branch_deleted". The "removed" reachability action (a branch that moved without disappearing, Line 34-41 ofcommit_branch_reachability.py) and the 404 retry/reset andremoval_unavailablefallback paths (Line 70-97 of the same file) have no test coverage, which lines up with the diff-coverage gate's flagged gaps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ingestion/connectors/git/bitbucket-cloud/tests/test_branches.py` around lines 28 - 45, Extend the reachability tests for CommitBranchReachabilityStream to cover a moved branch producing the “removed” action, plus the 404 retry/reset behavior and the removal_unavailable fallback in the commit branch reachability flow. Reuse the existing stream state and client fixtures, and assert each path’s emitted action and resulting state.Source: Pipeline failures
src/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/source.py (1)
130-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded stream count in log message.
"streams: wired 20 streams"is a magic literal that must be kept in sync by hand with the returned list (Line 134-155). Deriving it from the list itself avoids silent drift if a stream is added/removed later.♻️ Proposed fix
- _logger.info( - f"streams: wired 20 streams (workspaces={shared['workspaces']} " - f"start_date={shared['start_date']} skip_forks={shared['skip_forks']})" - ) - return [ + wired = [ repos, branches, prs, pr_diffstat, pr_activity, pr_tasks, pr_comments, pr_commits, pipelines, pipeline_steps, pipeline_step_test_reports, deployments, environments, tags, issues, issue_comments, issue_changes, commits, commit_branch_reachability, file_changes, ] + _logger.info( + f"streams: wired {len(wired)} streams (workspaces={shared['workspaces']} " + f"start_date={shared['start_date']} skip_forks={shared['skip_forks']})" + ) + return wired🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/source.py` around lines 130 - 133, Update the logging around the returned stream list in the source connector to derive the wired stream count from that list instead of hardcoding 20. Use the list length in the existing _logger.info message while preserving the workspace, start_date, and skip_forks details.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/client.py`:
- Around line 74-100: Add unit tests for the Bitbucket client reliability paths,
using a mocked requests.Session: cover request retry-then-success behavior,
_retry_delay parsing of Retry-After and X-RateLimit-Reset, paginate and
paginate_optional list/error branches including 403 and 404-then-success, and
repositories()/branches() response field mapping. Ensure the tests exercise the
transient failures, successful responses, and expected mapped results.
- Around line 147-175: Update the follow-up request in paginate_optional’s
records generator to pass the same allow_statuses={403, 404} used for the
initial page. Preserve the existing pagination and termination behavior so later
403/404 responses stop gracefully rather than raising BitbucketApiError.
In
`@src/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/commit_branch_reachability.py`:
- Around line 66-97: The _changes method’s 404 fallback can re-emit commits
already yielded before a later pagination failure. Update the fallback around
commits_between and _reachability_records to track or preserve pagination
progress, so the added/reset recovery emits only records not already yielded;
retain the existing removal_unavailable behavior for non-added actions. In
src/ingestion/connectors/git/bitbucket-cloud/streams/commit_branch_reachability.py
lines 66-97, implement the guard; in
src/ingestion/connectors/git/bitbucket-cloud/tests/test_branches.py lines 28-45,
add coverage for a paginated failure after records have been yielded.
In
`@src/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/git_ranges.py`:
- Around line 22-27: Add test coverage for the 404 recovery branch in the stream
logic around _client.commits_between: configure a non-empty previous_heads
value, make the first call raise BitbucketApiError with status_code 404, and
assert the retry succeeds with the same repo and current_heads but an empty
excludes list. Also verify the normal non-404 behavior remains unchanged by
allowing the exception to propagate.
---
Nitpick comments:
In `@src/ingestion/connectors/git/bitbucket-cloud/dbt/schema.yml`:
- Around line 24-32: Override the table-level freshness to null for the optional
resources pipelines, pipeline_steps, pipeline_step_test_reports, deployments,
environments, tags, and issues in the schema definitions. Leave issue_comments
and issue_changes unchanged unless they are also explicitly optional, preserving
the source-level 72-hour freshness SLA for required streams.
In
`@src/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/source.py`:
- Around line 130-133: Update the logging around the returned stream list in the
source connector to derive the wired stream count from that list instead of
hardcoding 20. Use the list length in the existing _logger.info message while
preserving the workspace, start_date, and skip_forks details.
In `@src/ingestion/connectors/git/bitbucket-cloud/tests/test_branches.py`:
- Around line 28-45: Extend the reachability tests for
CommitBranchReachabilityStream to cover a moved branch producing the “removed”
action, plus the 404 retry/reset behavior and the removal_unavailable fallback
in the commit branch reachability flow. Reuse the existing stream state and
client fixtures, and assert each path’s emitted action and resulting state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4fa0b6b-c6d8-42fa-9aec-1a12604b7d31
📒 Files selected for processing (39)
src/ingestion/connectors/git/bitbucket-cloud/.dockerignoresrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__bronze_promoted.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__commit_branch_reachability.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__commits.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__file_changes.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__pull_requests.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__pull_requests_comments.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__pull_requests_commits.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__pull_requests_reviewers.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__repositories.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/bitbucket_cloud__repository_branches.sqlsrc/ingestion/connectors/git/bitbucket-cloud/dbt/schema.ymlsrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/client.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/source.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/spec.jsonsrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/base.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/branches.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/commit_branch_reachability.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/commits.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/file_changes.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/git_ranges.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/metric_events.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/pr_activity.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/pr_base.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/pr_comments.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/pr_commits.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/pr_diffstat.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/pull_requests.pysrc/ingestion/connectors/git/bitbucket-cloud/source_bitbucket_cloud/streams/repositories.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/conftest.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_base.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_branches.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_commits.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_file_changes.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_pr_children.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_pull_requests.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_reliability.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_repositories.pysrc/ingestion/connectors/git/bitbucket-cloud/tests/test_source.py
- paginate_optional now stops gracefully when a later page returns 403/404 instead of raising, matching the first-page contract. - commit_branch_reachability buffers a branch range before emitting so a paginated 404 mid-range triggers the reset fallback without re-emitting the records already yielded. - Disable dbt source freshness on optional, permission- or feature-gated streams (pipelines, deployments, issue tracker, tags) whose bronze tables may legitimately stay empty and would otherwise raise false staleness. - Derive the wired-stream count in logging from the stream list. - Add coverage for client retry/backoff/pagination/mapping, new_commits and reachability 404 recovery, and the reachability partial-page guard. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
…ents Adds coverage for the previously untested metric-event streams — pipelines (including open-pipeline refetch), pipeline steps and step test reports, environments/tags/deployments snapshots, issues with the has_issues gate, issue comments/changes, and PR tasks — plus pull-request activity event-type classification. Raises connector line coverage from 78% to 94% and clears the per-component and new-code coverage gates. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
aleksdotbar
enabled auto-merge
July 22, 2026 22:09
mitasovr
approved these changes
Jul 23, 2026
This was referenced Jul 23, 2026
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Summary by CodeRabbit