Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎉 Source Hubspot: Migrate to CDK #10177

Merged
merged 80 commits into from Feb 21, 2022
Merged

Conversation

augan-rymkhan
Copy link
Contributor

@augan-rymkhan augan-rymkhan commented Feb 8, 2022

What

Resolves #5114
Refactor the connector to use CDK classes.

How

Make SourceHubspot inherits from AbstractSource

  • Implement check_connection, we can use the existing logic prodived by client.health_check in this method
  • Fix stream.name property so discover command can find the related schemas and add dynamic properties by overriding get_json_schema method. To do that refactor stream class names. Schema names should not be changed to support backward compatibilty.
  • Implement streams method, move logic in Client._apis to this method and change the its type from dict to list.
  • Remove Client class.

Refactor Stream classes

  • Make Stream base class inherit from airbyte_cdk.sources.streams.core import Stream
  • Mark IncrementalStream stream as support incremental by adding field cursor_field
  • Add method get_updated_state to IncrementalStream class, which returns self.state or {self.state_pk: self._start_date}
    IMO, we should not change state property because it has complex logic of handling stream state.
  • Add read_records method to Stream class, which runs generator list_records(fields=fields)
  • Override read_records method in IncrementalStream so it sets initial state got from connector state:
       if stream_state:
           self.state = stream_state
       yield from self.list_records(fields=self.get_fields())

Recommended reading order

  1. source-hubspot/source_hubspot/source.py
  2. source-hubspot/source_hubspot/api.py
  3. source-hubspot/source_hubspot/client.py
  4. source-hubspot/source_hubspot/test_source.py
  5. source-hubspot/acceptance-test-config.yml

@github-actions github-actions bot added the area/connectors Connector related issues label Feb 8, 2022
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 8, 2022 11:40 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 9, 2022 06:18 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 10, 2022 06:09 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 10, 2022 06:21 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 10, 2022 14:45 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 10, 2022 15:11 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 18, 2022 13:29 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 18, 2022 13:29 Inactive
@@ -34,8 +34,8 @@ tests:
future_state_path: "integration_tests/abnormal_state.json"
cursor_paths:
subscription_changes: ["timestamp"]
email_events: ["timestamp"]
contact_lists: ["timestamp"]
email_events: ["created"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it matches the cursor_field in a stream (catalog) then you don't need cusor_paths for this stream (and any of them)
the purpose of cursor_paths is to handle cases when the path in the STATE file is nested

"some_field": {
   "cursor_field": "value"
}

@augan-rymkhan
Copy link
Contributor Author

augan-rymkhan commented Feb 21, 2022

/test connector=connectors/source-hubspot

🕑 connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1874874532
❌ connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1874874532
🐛 https://gradle.com/s/qyjwtkpo6vlkk

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 21, 2022 07:52 Inactive
@augan-rymkhan
Copy link
Contributor Author

augan-rymkhan commented Feb 21, 2022

/test connector=connectors/source-hubspot

🕑 connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1876325344
❌ connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1876325344
🐛 https://gradle.com/s/pq7qkzqjx6sa6

@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 13:10 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 13:10 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 21, 2022 13:13 Inactive
@augan-rymkhan
Copy link
Contributor Author

augan-rymkhan commented Feb 21, 2022

/test connector=connectors/source-hubspot

🕑 connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1876927017
✅ connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1876927017
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  70     17    76%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              275    106    61%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  876    259    70%
Name                         Stmts   Miss  Cover
------------------------------------------------
source_hubspot/errors.py         6      0   100%
source_hubspot/__init__.py       2      0   100%
source_hubspot/streams.py      655    121    82%
source_hubspot/source.py        69     16    77%
------------------------------------------------
TOTAL                          732    137    81%

@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 15:14 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 15:14 Inactive
@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 21, 2022 15:15 Inactive
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Feb 21, 2022
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 15:41 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 15:41 Inactive
@augan-rymkhan
Copy link
Contributor Author

augan-rymkhan commented Feb 21, 2022

/publish connector=connectors/source-hubspot

🕑 connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1877074565
✅ connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/1877074565

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets February 21, 2022 15:46 Inactive
@augan-rymkhan augan-rymkhan merged commit 2282a4a into master Feb 21, 2022
@augan-rymkhan augan-rymkhan deleted the arymkhan/migrate-hubspot-to-cdk branch February 21, 2022 16:03
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 16:03 Inactive
@augan-rymkhan augan-rymkhan temporarily deployed to more-secrets February 21, 2022 16:03 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source Hubspot: migrate to CDK
5 participants