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: unnest data #30322

Merged
merged 13 commits into from
Sep 27, 2023

Conversation

davydov-d
Copy link
Collaborator

@davydov-d davydov-d commented Sep 11, 2023

What

#29345

How

  • Unnest static schemas
  • Unnest dynamic schemas
  • Unnest data itself to correspond the schemas
  • Update CAT and unit tests, bump version, update changelog

🚨 User Impact 🚨

Most of nested original data will be duplicated with unnested columns -- not a breaking change

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/hubspot labels Sep 11, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2023

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@davydov-d davydov-d marked this pull request as ready for review September 11, 2023 20:18
@github-actions
Copy link
Contributor

source-hubspot test report (commit f5e68805ad) - ❌

⏲️ Total pipeline duration: 13mn37s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@davydov-d davydov-d added the breaking-change Don't merge me unless you are ready. label Sep 12, 2023
@github-actions
Copy link
Contributor

source-hubspot test report (commit a3184a0e00) - ✅

⏲️ Total pipeline duration: 08mn48s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@lazebnyi lazebnyi requested a review from brianjlai September 18, 2023 09:31
@davydov-d davydov-d requested a review from a team September 19, 2023 15:40
@github-actions
Copy link
Contributor

source-hubspot test report (commit dde64d7fb4) - ❌

⏲️ Total pipeline duration: 12mn03s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@davydov-d
Copy link
Collaborator Author

@github-actions
Copy link
Contributor

source-hubspot test report (commit c8c1a7103c) - ❌

⏲️ Total pipeline duration: 10mn01s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@davydov-d
Copy link
Collaborator Author

acceptance tests are failing due to actual and expected items mismatch - will fix it soon

@github-actions
Copy link
Contributor

source-hubspot test report (commit 055c9a57a2) - ❌

⏲️ Total pipeline duration: 05mn53s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@github-actions
Copy link
Contributor

source-hubspot test report (commit 7dfc1bf937) - ✅

⏲️ Total pipeline duration: 05mn54s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

Copy link
Contributor

@brianjlai brianjlai left a comment

Choose a reason for hiding this comment

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

As per the issue I noticed that these schemas weren't modified. And that was because they are already de-nested?

  • line_items
  • products
  • property_history
  • tickets

The code looks good and I went over most of the schemas. We should add some unit tests for the new unnester class since it has some pretty specific logic as to how it performs the unnesting. Will approve once we add those tests in.

What's the release plan? Since it's a breaking change are we giving customers a warning first, and then a window for them to refresh their schema and data?

"type": ["null", "string"]
}
}
"properties_hs_all_assigned_business_unit_ids": {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if natalie corrected this, but i did noticed you asked. I assume it was a typo and she meant properties as you have it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if I understand this right, this column is named correctly. Please check out this answer and see if it answers your question

@vercel
Copy link

vercel bot commented Sep 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 25, 2023 6:54pm

@davydov-d davydov-d removed the breaking-change Don't merge me unless you are ready. label Sep 25, 2023
@davydov-d davydov-d changed the title 🚨 🚨 Source Hubspot: unnest data ✨ ✨ Source Hubspot: unnest data Sep 25, 2023
@airbyte-oss-build-runner
Copy link
Collaborator

source-hubspot test report (commit 9f1181256d) - ✅

⏲️ Total pipeline duration: 11mn57s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@davydov-d
Copy link
Collaborator Author

@brianjlai I have updated this PR to address your comments. Please check it out one more time.
Regarding your question about the four stream schemas that were not changed: they hardly have nested data except for the custom properties which are unnested dynamically. Also, the release plan is not needed as this is no more a breaking change. We decided to leave the original data as is, and duplicate it with the unnested columns.

@davydov-d davydov-d requested a review from brianjlai September 25, 2023 11:52
@airbyte-oss-build-runner
Copy link
Collaborator

source-hubspot test report (commit 6e72ef3aa5) - ❌

⏲️ Total pipeline duration: 11mn57s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@airbyte-oss-build-runner
Copy link
Collaborator

source-hubspot test report (commit f299c9bb64) - ✅

⏲️ Total pipeline duration: 13mn39s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

@airbyte-oss-build-runner
Copy link
Collaborator

source-hubspot test report (commit f2452adbbd) - ✅

⏲️ Total pipeline duration: 09mn40s

Step Result
Connector package install
Build source-hubspot docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-hubspot/metadata.yaml
Connector version semver check
Connector version increment check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-hubspot test

Copy link
Contributor

@brianjlai brianjlai left a comment

Choose a reason for hiding this comment

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

no changes on my end, this does feel less risky now that it is non-breaking and additive to the schema. Not a blocker, but do we plan to eventually deprecate these legacy nested fields. this could lead to almost double the size of some records which could impact performance or at least cost?

@davydov-d
Copy link
Collaborator Author

no changes on my end, this does feel less risky now that it is non-breaking and additive to the schema. Not a blocker, but do we plan to eventually deprecate these legacy nested fields. this could lead to almost double the size of some records which could impact performance or at least cost?

I am not aware of a plan like this. I think we should ask Kat or Natalie

@davydov-d davydov-d merged commit 8186e80 into master Sep 27, 2023
21 checks passed
@davydov-d davydov-d deleted the ddavydov/#29345-source-hubspot-unnest-data branch September 27, 2023 06:10
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 checklist-action-run connectors/source/hubspot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants