-
Notifications
You must be signed in to change notification settings - Fork 4.5k
CAT: add validation for state messages #36001
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good but I have a couple comments on the validations we have and I think adding a bypass_reason
like we do on some of our other tests might be good escape hatch. The default should be on however.
airbyte-integrations/bases/connector-acceptance-test/unit_tests/test_core.py
Outdated
Show resolved
Hide resolved
...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py
Show resolved
Hide resolved
...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the should_validate_state_messages
flag to disable the test when necessary.
Decide not to add bypass_reason
to avoid unnecessary complicity in configuration of this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! thanks for making the additions
What
Fixed https://github.com/airbytehq/airbyte-internal-issues/issues/6557
Emit final state message for full refresh syncs and consolidate read flows was added - #35622
We need to test that the legacy state type is not emitted and that the source stats are present in the state message
How
add validation for state messages
Recommended reading order
airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py
🚨 User Impact 🚨
no breaking changes
Pre-merge Actions
Updating the Python CDK
Airbyter
Before merging:
--use-local-cdk --name=source-<connector>
as optionsairbyte-ci connectors --use-local-cdk --name=source-<connector> test
After merging: