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

[ISSUE #35110] match CATs records only one primary key when primary k… #35556

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

maxi297
Copy link
Contributor

@maxi297 maxi297 commented Feb 22, 2024

…ey is defined

What

Addresses airbytehq/airbyte-internal-issues#6376

How

If source_defined_primary_key is defined, validate only primary key values. Handle the both case exact_order = true and exact_order = false

Test exact_order = False (stripe)

CATs are currently failing for stripe because subscription_items, subscription_schedule, subscriptions and usage_records are now empty so in order to test, I did:

  • add subscription_items, subscription_schedule, subscriptions and usage_records as empty streams
  • change txn_1KVQhfEcXtiJtvvhF7ox3YEm to txn_1KQhfEcXtiJtvvhF7ox3YEm for balance_transactions
  • ran python -m pytest integration_tests -p integration_tests.acceptance -k TestBasicRead

Outcome:
Screenshot 2024-02-22 at 3 37 38 PM
Note: the stream name was added since the printscreen was taken

Test exact_order = True (google-ads)

In order to test, I did:

  • inverse the order of the first two ad_group_ad_legacy expected records
  • ran python -m pytest integration_tests -p integration_tests.acceptance -k TestBasicRead

Outcome:
Screenshot 2024-02-22 at 3 34 13 PM
Note: a specific assertion message has been added since the printscreen was taken

If someone wants more validation, I can change one of the primary key fields value or any other tests.

🚨 User Impact 🚨

This is a relaxation of CATs. Hence, we expect more CATs to start passing and CATs to be less flaky. The drawback for now is is that we will lack visibility on new fields that are not added on the root level (root level is validated by Datadog) as NoAdditionalPropertiesValidator will not be used anymore until we have a better plan for schema validation.

@maxi297 maxi297 requested a review from a team February 22, 2024 20:48
Copy link

vercel bot commented Feb 22, 2024

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Feb 27, 2024 6:24pm

@maxi297 maxi297 requested a review from alafanechere February 22, 2024 20:51
@maxi297 maxi297 requested a review from ChristoGrab February 22, 2024 20:54
Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

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

minor suggestions, I'll let you decide if they're worth it. LGTM 👍 tried out on #35587 : it's working.

@maxi297 maxi297 merged commit 7baf154 into master Feb 27, 2024
27 checks passed
@maxi297 maxi297 deleted the issue-35110/relax-cats-when-primary-key branch February 27, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants