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: fix acceptance tests #36541

Merged
merged 3 commits into from Apr 8, 2024

Conversation

roman-yermilov-gl
Copy link
Collaborator

What

Disable CAT state validation test due to failing tests. The source have complex conflicts with the latest CDK version, so the new test is temporarily disabled as a workaround. Reduced time of entire sync to avoid timeouts during acceptance test run

Copy link

vercel bot commented Mar 27, 2024

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 Apr 8, 2024 2:25pm

@roman-yermilov-gl roman-yermilov-gl changed the title Ryermilov/source hubspot fix acceptance tests Source Hubspot: fix acceptance tests Mar 27, 2024
@roman-yermilov-gl roman-yermilov-gl marked this pull request as ready for review March 27, 2024 12:31
@octavia-squidington-iv octavia-squidington-iv requested review from a team March 27, 2024 12:32
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-hubspot-fix-acceptance-tests branch from 3a0acc2 to ccf7ba4 Compare March 27, 2024 13:00
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Mar 27, 2024
@@ -10,7 +10,7 @@ data:
connectorSubtype: api
connectorType: source
definitionId: 36c891d9-4bd9-43ac-bad2-10e12756272c
dockerImageTag: 4.0.0
dockerImageTag: 4.0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Please set this to 4.1.0 due to airbyte-cdk = "^0" change

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.

Disable CAT state validation test due to failing tests. The source have complex conflicts with the latest CDK version, so the new test is temporarily disabled as a workaround. Reduced time of entire sync to avoid timeouts during acceptance test run

I'm curious to dig into this a little bit. Was there a specific version of the CDK was posing a problem? I noticed we are bumping using "^0" so it sounds like we're not concerned about pinning to a specific version. So presumably poetry lock was run back when the CDK package was only at 0.73.0. Can we rerun this command and push the change to the lockfile to the PR?

Which part of the validate_state_messages was failing? Was it the verifying the recordCounts field was on the state message, or the requirement that every stream emit at least one final state message.

@octavia-squidington-iv octavia-squidington-iv requested a review from a team March 28, 2024 22:37
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-hubspot-fix-acceptance-tests branch from 01041ea to dd10a53 Compare April 3, 2024 17:03
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-hubspot-fix-acceptance-tests branch 2 times, most recently from 5b783d8 to f915089 Compare April 3, 2024 17:07
@roman-yermilov-gl roman-yermilov-gl force-pushed the ryermilov/source-hubspot-fix-acceptance-tests branch from f915089 to 3c4bbb3 Compare April 4, 2024 11:40
@roman-yermilov-gl
Copy link
Collaborator Author

Disable CAT state validation test due to failing tests. The source have complex conflicts with the latest CDK version, so the new test is temporarily disabled as a workaround. Reduced time of entire sync to avoid timeouts during acceptance test run

I'm curious to dig into this a little bit. Was there a specific version of the CDK was posing a problem? I noticed we are bumping using "^0" so it sounds like we're not concerned about pinning to a specific version. So presumably poetry lock was run back when the CDK package was only at 0.73.0. Can we rerun this command and push the change to the lockfile to the PR?

Which part of the validate_state_messages was failing? Was it the verifying the recordCounts field was on the state message, or the requirement that every stream emit at least one final state message.

Update: made poetry lock and deleted validate_state_messages. In order to incremental tests to pass I have also prepared PR: #36814 with fix of the test itself

@lazebnyi
Copy link
Collaborator

lazebnyi commented Apr 8, 2024

follow up - #36814

@lazebnyi lazebnyi merged commit 0e8a9a9 into master Apr 8, 2024
28 of 29 checks passed
@lazebnyi lazebnyi deleted the ryermilov/source-hubspot-fix-acceptance-tests branch April 8, 2024 14:40
markcusack pushed a commit to markcusack/airbyte that referenced this pull request Apr 9, 2024
Co-authored-by: Serhii Lazebnyi <53845333+lazebnyi@users.noreply.github.com>
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 connectors/source/hubspot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants