fix(warehouse-sources): skip HubSpot objects the grant cannot read - #95301
Merged
Conversation
The properties, pipelines and pipeline_stages tables read one path per object type, so an object the grant cannot read must not fail the whole table. #74909 made a 403 raise HubspotForbiddenError, which is not a requests.HTTPError, so the skip in _iter_pages stopped catching it and one unreadable object failed every sync of the table. Catch HubspotForbiddenError, which also covers the missing-scope subclass, and skip only where the caller fans out. Owners reads a single endpoint, so it keeps the 403 and asks the customer to reconnect. The existing test mocked fetch_data and raised requests.HTTPError by hand, so it never exercised the status mapping and stayed green through the regression. The new cases mock the HTTP session instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
😎 Merged successfully - details. |
Contributor
🤖 CI report
|
Contributor
|
Reviews (1): Last reviewed commit: "fix(warehouse-sources): restore the HubS..." | Re-trigger Greptile |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
Contained bug fix within the owning team's own module, backed by tests that exercise the real HTTP-to-exception mapping; no schema, API, auth, or dependency surface is touched, and no outstanding review concerns exist.
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 22L, 1F substantive, 81L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (81L, 2F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 8ee4cc3 · reviewed head 8ee4cc3 |
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
1 participant
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.
Problem
A HubSpot properties table fails on every sync when the portal's grant cannot read one of HubSpot's object types. Reconnecting does not fix it.
HUBSPOT_ENDPOINTS, so one unreadable object type fails the whole table._iter_pagesalready means to tolerate this, and its guard catchesrequests.HTTPError.HubspotForbiddenError, which is not arequests.HTTPError, so the guard stopped catching it.Changes
_iter_pagestakes a keyword-onlyskip_forbiddenand catchesHubspotForbiddenError. The catch covers theHubspotMissingScopeErrorsubclass, so a scope-gated object such as leads no longer fails the table._SKIPPABLE_STATUSES.raise_for_hubspot_statusintercepts a 403 beforeraise_for_status, so that branch could never see one.Nothing a person sees changes apart from the sync outcome, so there are no screenshots.
How did you test this code?
The previous test patched
fetch_dataand raisedrequests.HTTPErrorby hand. It never ran the status mapping, which is why it stayed green through the regression. The new cases patch the HTTP session, so the real mapping runs.test_a_fan_out_skips_an_object_type_the_grant_cannot_readcovers four cases: properties with a plain 403, properties with the scope-gated leads 403, pipelines, and pipeline_stages. It catches a call site that misses the skip, which is the shape of this bug.test_owners_fails_the_table_when_the_grant_cannot_read_itcatches a change that widens the skip to single-endpoint tables.All four fan-out cases fail against the unfixed
metadata.pyand pass with it, checked locally by reverting that file and re-running.Not done: no run against a live HubSpot portal.
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude Code (Opus 5). Skills invoked:
/triaging-warehouse-sync-tickets,/writing-tests,/writing-pr-descriptions.Duplicate found: #95236 fixes the same root cause in the same two files and opened a few hours earlier. It differs on one decision. It skips the 403 for every lookup table, owners included, which turns an unreadable owners grant into an empty table that reports success. This PR keeps owners failing, which is what the reporting customer's own case supports. It also covers pipelines and pipeline_stages, and patches the HTTP session rather than hand-calling
raise_for_hubspot_statusinside a patchedfetch_data. Merge one of the two, not both.Rejected alternative: gating the fan-out on the schemas selected for sync. It needs a database read inside the fetcher, changes what the properties table contains, and still misses an object whose scope is never requested.
Public artifact: the investigation drew on a customer support ticket and that customer's production sync logs. Nothing from either appears in this PR. No fixture is derived from customer data, and the tests use object-type names already in the source.