fix(source-hubspot): bump to latest version of CDK to get the bug fix for missing custom properties during incremental syncs#68159
Conversation
…, add test to validate incremental query properties, fix tests
Brian Lai (brianjlai)
requested review from
a team and
Maxime Carbonneau-Leclerc (maxi297)
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Helpful Resources
PR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
|
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
vale
docs/integrations/sources/hubspot.md|439 col 104| [Vale.Spelling] Did you really mean '_parse_and_handle_errors'?
docs/integrations/sources/hubspot.md|441 col 90| [Vale.Spelling] Did you really mean 'autopull'?
docs/integrations/sources/hubspot.md|442 col 89| [Vale.Spelling] Did you really mean 'etry'?
docs/integrations/sources/hubspot.md|444 col 122| [Google.Spacing] 'g.L' should have one space.
docs/integrations/sources/hubspot.md|446 col 89| [Vale.Spelling] Did you really mean 'contacts_list_memberships'?
docs/integrations/sources/hubspot.md|457 col 94| [Vale.Spelling] Did you really mean 'property_history'?
docs/integrations/sources/hubspot.md|463 col 118| [Vale.Spelling] Did you really mean 'Dockerfile'?
docs/integrations/sources/hubspot.md|466 col 89| [Vale.Spelling] Did you really mean 'Unnest'?
docs/integrations/sources/hubspot.md|480 col 121| [Vale.Spelling] Did you really mean 'boolean'?
docs/integrations/sources/hubspot.md|481 col 96| [Vale.Spelling] Did you really mean 'authSpecification'?
docs/integrations/sources/hubspot.md|481 col 156| [Vale.Spelling] Did you really mean 'advancedAuth'?
docs/integrations/sources/hubspot.md|486 col 151| [Vale.Spelling] Did you really mean 'settip'?
docs/integrations/sources/hubspot.md|497 col 89| [Vale.Spelling] Did you really mean 'Sunsetting'?
docs/integrations/sources/hubspot.md|511 col 108| [Vale.Spelling] Did you really mean 'http'?
docs/integrations/sources/hubspot.md|511 col 128| [Google.WordList] Use 'turn off' or 'off' instead of 'disable'.
docs/integrations/sources/hubspot.md|529 col 108| [Vale.Spelling] Did you really mean 'ticket_pipeline'?
docs/integrations/sources/hubspot.md|530 col 99| [Vale.Spelling] Did you really mean 'slep'?
docs/integrations/sources/hubspot.md|531 col 109| [Vale.Spelling] Did you really mean 'yaml'?
docs/integrations/sources/hubspot.md|536 col 108| [Vale.Spelling] Did you really mean 'upd'?
docs/integrations/sources/hubspot.md|547 col 97| [Vale.Spelling] Did you really mean 'form_submission'?
docs/integrations/sources/hubspot.md|552 col 104| [Vale.Spelling] Did you really mean 'form_submissions'?
docs/integrations/sources/hubspot.md|558 col 97| [Vale.Spelling] Did you really mean 'skip_dynamic_fields'?
docs/integrations/sources/hubspot.md|558 col 125| [Vale.Spelling] Did you really mean 'param'?
docs/integrations/sources/hubspot.md|564 col 89| [Vale.Spelling] Did you really mean 'Hotfix'?
docs/integrations/sources/hubspot.md|567 col 115| [Google.WordList] Use 'turn off' or 'off' instead of 'disable'.
docs/integrations/sources/hubspot.md|570 col 97| [Vale.Spelling] Did you really mean 'deal_to_contact'?
| | 6.0.5 | 2025-10-07 | [67400](https://github.com/airbytehq/airbyte/pull/67400) | Update dependencies | | ||
| | 6.0.4 | 2025-09-30 | [66409](https://github.com/airbytehq/airbyte/pull/66409) | Update dependencies | | ||
| | 6.0.3 | 2025-09-25 | [66700](https://github.com/airbytehq/airbyte/pull/66700) | Revert to 6.0.1 | | ||
| | 6.0.2 | 2025-09-12 | [65947](https://github.com/airbytehq/airbyte/pull/65947) | Set fallback target type to 'string' for numbers and booleans | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'booleans'?
| | 5.8.18 | 2025-08-09 | [64603](https://github.com/airbytehq/airbyte/pull/64603) | Update dependencies | | ||
| | 5.8.17 | 2025-08-02 | [64197](https://github.com/airbytehq/airbyte/pull/64197) | Update dependencies | | ||
| | 5.8.16 | 2025-07-26 | [63898](https://github.com/airbytehq/airbyte/pull/63898) | Update dependencies | | ||
| | 5.8.15 | 2025-07-21 | [63341](https://github.com/airbytehq/airbyte/pull/63341) | Bump memory on Discover to 1GB | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Google.Units] Put a nonbreaking space between the number and the unit in '1GB'.
| | 5.8.13 | 2025-07-12 | [63115](https://github.com/airbytehq/airbyte/pull/63115) | Update dependencies | | ||
| | 5.8.12 | 2025-07-08 | [62866](https://github.com/airbytehq/airbyte/pull/62866) | Handle non-numeric values in fields with declared numeric type | | ||
| | 5.8.11 | 2025-07-07 | [62838](https://github.com/airbytehq/airbyte/pull/62838) | Promoting release candidate 5.8.11-rc.1 to a main version. | | ||
| | 5.8.11-rc.1 | 2025-07-02 | [62481](https://github.com/airbytehq/airbyte/pull/62481) | For CRMSearch streams, fix retry behavior for the underlying associations HttpRequester to retry 401 errors | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'CRMSearch'?
| | 5.7.0 | 2025-05-27 | [60919](https://github.com/airbytehq/airbyte/pull/60919) | Promoting release candidate 5.7.0-rc.2 to a main version. | | ||
| | 5.7.0-rc.2 | 2025-05-23 | [60881](https://github.com/airbytehq/airbyte/pull/60881) | Ignore 403 errors for dynamic streams to prevent sync failures | | ||
| | 5.7.0-rc.1 | 2025-05-22 | [60830](https://github.com/airbytehq/airbyte/pull/60830) | Migrate CustomObjects streams | | ||
| | 5.6.0 | 2025-05-21 | [59727](https://github.com/airbytehq/airbyte/pull/60338) | Migrate CRM search streams: contacts, deal_splits, leads, tickets | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'deal_splits'?
| | 5.7.0-rc.1 | 2025-05-22 | [60830](https://github.com/airbytehq/airbyte/pull/60830) | Migrate CustomObjects streams | | ||
| | 5.6.0 | 2025-05-21 | [59727](https://github.com/airbytehq/airbyte/pull/60338) | Migrate CRM search streams: contacts, deal_splits, leads, tickets | | ||
| | 5.5.0 | 2025-05-21 | [60812](https://github.com/airbytehq/airbyte/pull/60812) | Migrate CRM search companies stream | | ||
| | 5.4.0 | 2025-05-20 | [59727](https://github.com/airbytehq/airbyte/pull/59727) | Migrate CRM object streams: goals, product, line_items | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'line_items'?
| | 4.6.4 | 2025-04-22 | [58138](https://github.com/airbytehq/airbyte/pull/58138) | Use short-hand custom object type name path for custom object streams. | | ||
| | 4.6.3 | 2025-04-19 | [58226](https://github.com/airbytehq/airbyte/pull/58226) | Update dependencies | | ||
| | 4.6.2 | 2025-04-18 | [58137](https://github.com/airbytehq/airbyte/pull/58137) | Promoting release candidate 4.6.2-rc.1 to a main version. | | ||
| | 4.6.2-rc.1 | 2025-04-13 | [57534](https://github.com/airbytehq/airbyte/pull/57534) | Migrate marketing_emails and email_subscriptions to low code | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'email_subscriptions'?
| | 4.6.0-rc.1 | 2025-03-31 | [56919](https://github.com/airbytehq/airbyte/pull/56919) | Update CDK to v6 | | ||
| | 4.5.6 | 2025-03-29 | [56647](https://github.com/airbytehq/airbyte/pull/56647) | Update dependencies | | ||
| | 4.5.5 | 2025-03-26 | [56416](https://github.com/airbytehq/airbyte/pull/56416) | Disabled `blog_comment` and `all` form-types for `Forms` and `FormSubmissions` stream | | ||
| | 4.5.4 | 2025-03-25 | [55822](https://github.com/airbytehq/airbyte/pull/55822) | add userIdIncludingInactive to owners and owners_archived streams | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'owners_archived'?
| | 4.5.6 | 2025-03-29 | [56647](https://github.com/airbytehq/airbyte/pull/56647) | Update dependencies | | ||
| | 4.5.5 | 2025-03-26 | [56416](https://github.com/airbytehq/airbyte/pull/56416) | Disabled `blog_comment` and `all` form-types for `Forms` and `FormSubmissions` stream | | ||
| | 4.5.4 | 2025-03-25 | [55822](https://github.com/airbytehq/airbyte/pull/55822) | add userIdIncludingInactive to owners and owners_archived streams | | ||
| | 4.5.3 | 2025-03-24 | [52729](https://github.com/airbytehq/airbyte/pull/55245) | Add conversionId to `form_submissions` schema | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'conversionId'?
| | 4.5.5 | 2025-03-26 | [56416](https://github.com/airbytehq/airbyte/pull/56416) | Disabled `blog_comment` and `all` form-types for `Forms` and `FormSubmissions` stream | | ||
| | 4.5.4 | 2025-03-25 | [55822](https://github.com/airbytehq/airbyte/pull/55822) | add userIdIncludingInactive to owners and owners_archived streams | | ||
| | 4.5.3 | 2025-03-24 | [52729](https://github.com/airbytehq/airbyte/pull/55245) | Add conversionId to `form_submissions` schema | | ||
| | 4.5.2 | 2024-12-14 | [48480](https://github.com/airbytehq/airbyte/pull/49141) | Improves Forms stream by adding the formTypes query parameter to support all form types. This enhancement also propagates to the FormSubmission stream. | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'formTypes'?
| | 4.4.4 | 2024-12-21 | [50138](https://github.com/airbytehq/airbyte/pull/50138) | Update dependencies | | ||
| | 4.4.3 | 2024-12-14 | [48984](https://github.com/airbytehq/airbyte/pull/48984) | Update dependencies | | ||
| | 4.4.2 | 2024-12-10 | [48480](https://github.com/airbytehq/airbyte/pull/48480) | Adds individual read scopes to LineItems Stream | | ||
| | 4.4.1 | 2024-11-25 | [48662](https://github.com/airbytehq/airbyte/pull/48662) | Starting with this version, the Docker image is now rootless. Please note that this and future versions will not be compatible with Airbyte versions earlier than 0.64 | |
There was a problem hiding this comment.
[Google.Will] Avoid using 'will'.
| with_oauth=True, | ||
| with_dynamic_schemas=True, | ||
| entities=OBJECTS_WITH_DYNAMIC_SCHEMA, | ||
| entities=["calls"], |
There was a problem hiding this comment.
Before in v6 CDK we would evaluate all the json schemas when we call ConcurrentDeclarativeSource.schemas() which required that we mock every dynamic schema stream. But now in CDK v7 we lazily retrieve the json schema and make the outbound API requests for the streams defined in the catalog.
So now we don't need to mock every /properties call and if we did, this test would fail with mocks that were not triggered
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit 0799d5c. |
| | 4.2.7 | 2024-06-25 | [40441](https://github.com/airbytehq/airbyte/pull/40441) | Update dependencies | | ||
| | 4.2.6 | 2024-06-22 | [40126](https://github.com/airbytehq/airbyte/pull/40126) | Update dependencies | | ||
| | 4.2.5 | 2024-06-17 | [39432](https://github.com/airbytehq/airbyte/pull/39432) | Remove references to deprecated state method | | ||
| | 4.2.4 | 2024-06-10 | [38800](https://github.com/airbytehq/airbyte/pull/38800) | Retry hubspot _parse_and_handle_errors on JSON decode errors | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean '_parse_and_handle_errors'?
| | 4.2.5 | 2024-06-17 | [39432](https://github.com/airbytehq/airbyte/pull/39432) | Remove references to deprecated state method | | ||
| | 4.2.4 | 2024-06-10 | [38800](https://github.com/airbytehq/airbyte/pull/38800) | Retry hubspot _parse_and_handle_errors on JSON decode errors | | ||
| | 4.2.3 | 2024-06-06 | [39314](https://github.com/airbytehq/airbyte/pull/39314) | Added missing schema types for the `Workflows` stream schema | | ||
| | 4.2.2 | 2024-06-04 | [38981](https://github.com/airbytehq/airbyte/pull/38981) | [autopull] Upgrade base image to v1.2.1 | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'autopull'?
| | 4.2.4 | 2024-06-10 | [38800](https://github.com/airbytehq/airbyte/pull/38800) | Retry hubspot _parse_and_handle_errors on JSON decode errors | | ||
| | 4.2.3 | 2024-06-06 | [39314](https://github.com/airbytehq/airbyte/pull/39314) | Added missing schema types for the `Workflows` stream schema | | ||
| | 4.2.2 | 2024-06-04 | [38981](https://github.com/airbytehq/airbyte/pull/38981) | [autopull] Upgrade base image to v1.2.1 | | ||
| | 4.2.1 | 2024-05-30 | [38024](https://github.com/airbytehq/airbyte/pull/38024) | etry when attempting to get scopes | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'etry'?
| | 4.2.2 | 2024-06-04 | [38981](https://github.com/airbytehq/airbyte/pull/38981) | [autopull] Upgrade base image to v1.2.1 | | ||
| | 4.2.1 | 2024-05-30 | [38024](https://github.com/airbytehq/airbyte/pull/38024) | etry when attempting to get scopes | | ||
| | 4.2.0 | 2024-05-24 | [38049](https://github.com/airbytehq/airbyte/pull/38049) | Add resumable full refresh support to `contacts_form_submissions` and `contacts_merged_audit` streams | | ||
| | 4.1.5 | 2024-05-17 | [38243](https://github.com/airbytehq/airbyte/pull/38243) | Replace AirbyteLogger with logging.Logger | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Google.Spacing] 'g.L' should have one space.
| | 4.2.0 | 2024-05-24 | [38049](https://github.com/airbytehq/airbyte/pull/38049) | Add resumable full refresh support to `contacts_form_submissions` and `contacts_merged_audit` streams | | ||
| | 4.1.5 | 2024-05-17 | [38243](https://github.com/airbytehq/airbyte/pull/38243) | Replace AirbyteLogger with logging.Logger | | ||
| | 4.1.4 | 2024-05-16 | [38286](https://github.com/airbytehq/airbyte/pull/38286) | Added default schema normalization for the `Tickets` stream, to ensure the data types | | ||
| | 4.1.3 | 2024-05-13 | [38128](https://github.com/airbytehq/airbyte/pull/38128) | contacts_list_memberships as semi-incremental stream | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'contacts_list_memberships'?
| | 0.1.29 | 2021-12-17 | [8699](https://github.com/airbytehq/airbyte/pull/8699) | Add incremental sync support for `companies`, `contact_lists`, `contacts`, `deals`, `line_items`, `products`, `quotes`, `tickets` streams | | ||
| | 0.1.28 | 2021-12-15 | [8429](https://github.com/airbytehq/airbyte/pull/8429) | Update fields and descriptions | | ||
| | 0.1.27 | 2021-12-09 | [8658](https://github.com/airbytehq/airbyte/pull/8658) | Fix config backward compatibility issue by allowing additional properties in the spec | | ||
| | 0.1.26 | 2021-11-30 | [8329](https://github.com/airbytehq/airbyte/pull/8329) | Remove 'skip_dynamic_fields' config param | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'skip_dynamic_fields'?
| | 0.1.29 | 2021-12-17 | [8699](https://github.com/airbytehq/airbyte/pull/8699) | Add incremental sync support for `companies`, `contact_lists`, `contacts`, `deals`, `line_items`, `products`, `quotes`, `tickets` streams | | ||
| | 0.1.28 | 2021-12-15 | [8429](https://github.com/airbytehq/airbyte/pull/8429) | Update fields and descriptions | | ||
| | 0.1.27 | 2021-12-09 | [8658](https://github.com/airbytehq/airbyte/pull/8658) | Fix config backward compatibility issue by allowing additional properties in the spec | | ||
| | 0.1.26 | 2021-11-30 | [8329](https://github.com/airbytehq/airbyte/pull/8329) | Remove 'skip_dynamic_fields' config param | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'param'?
| | 0.1.23 | 2021-11-08 | [7730](https://github.com/airbytehq/airbyte/pull/7730) | Fix OAuth flow schema | | ||
| | 0.1.22 | 2021-11-03 | [7562](https://github.com/airbytehq/airbyte/pull/7562) | Migrate Hubspot source to CDK structure | | ||
| | 0.1.21 | 2021-10-27 | [7405](https://github.com/airbytehq/airbyte/pull/7405) | Change of package `import` from `urllib` to `urllib.parse` | | ||
| | 0.1.20 | 2021-10-26 | [7393](https://github.com/airbytehq/airbyte/pull/7393) | Hotfix for `split_properties` function, add the length of separator symbol `,`(`%2C` in HTTP format) to the checking of the summary URL length | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'Hotfix'?
| | 0.1.20 | 2021-10-26 | [7393](https://github.com/airbytehq/airbyte/pull/7393) | Hotfix for `split_properties` function, add the length of separator symbol `,`(`%2C` in HTTP format) to the checking of the summary URL length | | ||
| | 0.1.19 | 2021-10-26 | [6954](https://github.com/airbytehq/airbyte/pull/6954) | Fix issue with getting `414` HTTP error for streams | | ||
| | 0.1.18 | 2021-10-18 | [5840](https://github.com/airbytehq/airbyte/pull/5840) | Add new marketing emails (with statistics) stream | | ||
| | 0.1.17 | 2021-10-14 | [6995](https://github.com/airbytehq/airbyte/pull/6995) | Update `discover` method: disable `quotes` stream when using OAuth config | |
There was a problem hiding this comment.
[Google.WordList] Use 'turn off' or 'off' instead of 'disable'.
| | 0.1.17 | 2021-10-14 | [6995](https://github.com/airbytehq/airbyte/pull/6995) | Update `discover` method: disable `quotes` stream when using OAuth config | | ||
| | 0.1.16 | 2021-09-27 | [6465](https://github.com/airbytehq/airbyte/pull/6465) | Implement OAuth support. Use CDK authenticator instead of connector specific authenticator | | ||
| | 0.1.15 | 2021-09-23 | [6374](https://github.com/airbytehq/airbyte/pull/6374) | Use correct schema for `owners` stream | | ||
| | 0.1.14 | 2021-09-08 | [5693](https://github.com/airbytehq/airbyte/pull/5693) | Include deal_to_contact association when pulling deal stream and include contact ID in contact stream | |
There was a problem hiding this comment.
🚫 [vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'deal_to_contact'?
Maxime Carbonneau-Leclerc (maxi297)
left a comment
Maxime Carbonneau-Leclerc (maxi297)
left a comment
There was a problem hiding this comment.
LGTM! Just nit comments
|
|
||
|
|
||
| @freezegun.freeze_time("2022-02-25T17:00:00Z") | ||
| def test_crm_search_streams_requests_contain_custom_properties(requests_mock, fake_properties_list, config): |
There was a problem hiding this comment.
I'm fine adding this test too but wouldn't this be covered if we were to match the HTTP body for the POST request in test_leads_incremental?
There was a problem hiding this comment.
so I deleted test_leads_incremental now because that test was actually an accidental duplicate. So this is actually the intended test I wanted and I deleted test_leads_incremental
|
regression test results https://github.com/airbytehq/airbyte/actions/runs/18608057727
https://github.com/airbytehq/airbyte/actions/runs/18608067202
Conclusion: Records match up, but will do one last set of validation on both candidates that records contain all the dynamic properties. |
|
/approve-regression-tests see above comment on validated regression test results
|
|
confirmed w/ a local run that properties are back in the final emitted records. merging and starting progressive rollout |
Brian Lai (brianjlai)
deleted the
brian/hubspot_bump_version_for_query_properties_add_mock_server_test
branch
3 participants
What
We had a bug where during incremental syncs Hubspot CRM Search streams were not including the custom properties in the json body of the POST request so they were not getting received and emitted with records.
How
The bug was in the CDK and it was fixed in version
7.3.7in this airbytehq/airbyte-python-cdk#797We need to bump the version of SDM to get the fix, but in addition, we need to upgrade the unit_test
pyproject.tomlwhich is still on v6. I've also added a new test that validates that properties are indeed populated in the outbound request. And with the bump from v6 to v7 I fixed the tests which have now changed.Note: It does feel like we have something of a gap where our unit tests don't properly test CDK changes since the two are independently versioned... This is something we may want to investigate and solve so these types of things don't happen again
Can this PR be safely reverted and rolled back?
Kind of... If we do this wrong then we have to reset customers back to their previous state, but this is no different than the state we were previously in