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

SAT: backward compatibility - check that cursor fields were not changed #15520

Merged
merged 7 commits into from
Aug 11, 2022

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Aug 10, 2022

What

Closes #15439

We want to make backward compatibility test fail if a default_cursor_field value was changed in a stream of the connector catalog.

How

  • Retrieve previous and current catalog with a discover run (previously implemented in SAT: compatibility tests for catalogs #15486)
  • Refacto:
    • BaseDiffChecker and its child classes now perform the diff computation within the class
    • Multiple diff can be computed on different dict, so different diff can be used in the check_* function. This is useful because checking default_cursor_value change does not happen a the same diff as json_schema change.
    • Remove the diff computation from test_core.py
  • Add CatalogDiffChecker.check_if_cursor_field_was_changed with alongside test cases in test_backward_compatibility.FAILING_CATALOG_TRANSITIONS
  • Bump SAT to 0.2.0 as I consider this backward compatibility effort (on syntactic validation) done.

Recommended reading order

  1. test_backward_compatibility.py
  2. backward_compatibility.py
  3. test_core.py

🚨 User Impact 🚨

Developers changing the default_cursor_field to another value will face SAT failure.

@alafanechere alafanechere marked this pull request as ready for review August 10, 2022 17:14
@alafanechere alafanechere requested a review from a team August 10, 2022 17:14
@alafanechere alafanechere changed the title SAT: backward compatibility checks - no cursor changed on stream SAT: backward compatibility - check that cursor fields were not changed Aug 10, 2022
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

lgtm!

@alafanechere
Copy link
Contributor Author

alafanechere commented Aug 11, 2022

/publish connector=bases/source-acceptance-test auto-bump-version=false

🕑 Publishing the following connectors:
bases/source-acceptance-test
https://github.com/airbytehq/airbyte/actions/runs/2839785811


Connector Did it publish? Were definitions generated?
bases/source-acceptance-test

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@alafanechere alafanechere merged commit 51894a0 into master Aug 11, 2022
@alafanechere alafanechere deleted the augustin/sat/check-cursor-change branch August 11, 2022 12:53
girarda added a commit that referenced this pull request Aug 12, 2022
* greenhouse minus pagination

* jobs

* first substream

* rename field

* applications_demographics_answers_stream

* interviews

* All streams are implemented

* fix check

* fix spec

* disable backward compatibility tests

* disable backward compatibility tests

* unit tests

* definitions

* only use config.json

* bump version

* expected records

* delete stream classes

* Handle extracting no records from root

* handle missing keys

* Remove unused field from JsonSchema (#15425)

* few fixes from working with sendgrid

* reset to master

* only update the docstring

* reset

* 🎉 Source File - add support for custom encoding (#15293)

* added support for custom encoding

* fixed unit test for utf16

* updated docs

* bumped connector version

* auto-bump connector version [ci skip]

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

* change query frequency to 1hour (#15499)

* [low-code connectors]: Assert there are no custom top-level fields (#15489)

* move components to definitions field

* Also update the references

* validate the top level fields and add version

* raise exception on unknown fields

* newline

* unit tests

* set version to 0.1.0

* newline

* 🐞 Postgres source: fix bug in intermediate state emission (#15496)

* Rename record counter

* Rename method

* Emit intermediate state after all cursor records

* Emit intermediate state only when it is ready

* Merge two checks

* Add a testing message

* Fix unit tests

* Add one more testing record and add comments

* Add test case for multiple records with the same cursor value

* Revert irrelevant change

* Add explanation in javadoc

* Format code

* Rename testing methods

* Fix comment

* Bump version

* auto-bump connector version [ci skip]

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

* 🪟 🔧 Add testing and storybook component for CatalogDiffModal (#15426)

* wip diff modal test setup

* starting storybook add

* storybook working now

* cleanup

* aria labels

* test syncmode string

* 🎉 New Source: Hubplanner (#15521)

* added hubplanner source connector

* feat: added more streams to read from

* cleaned up some unneeded integration tests

* fix hubplanner schema

* changes

* update dockerfile

* add seed and doc

* update spec

* run source spec seed file

Co-authored-by: Ricky Renner <renner@amendllc.com>

* Make it possible to specify normalization pod resources. (#15495)

Today we are running into OOM exceptions with normalization. Normalization itself also inherits the destination's resource requirements. After work to bring destination memory usage down, this is no longer ideal, since most destinations use less memory than normalization needs.

This PR makes it possible to specify the general resource the normalization pod is provided via env vars.

Notes:
- Add env vars. Default to the various job main container resources if these are not set.
- Instead of using the destination's memory, use the normalization specify env vars.

* [low-code connectors] Extract datetime parser and handle %s format directive (#15429)

* fix parse

* Revert "fix parse"

This reverts commit 3c76c5a.

* fix parse timestamp

* extract datetime parser

* remove print

* use parser

* top level docstring

* rename variable

* do not use timestamp()

* Revert "do not use timestamp()"

This reverts commit 016cb69.

* update comment

* bump cdk version

* Update template

* source-file-secure bump to 0.2.16 (#15528)

* update Dockerfile version

* update init to accept additional args

* unit test sendgrid messages stream (#15331)

* unit test sendgrid messages stream

* reset

* Update airbyte-integrations/connectors/source-sendgrid/unit_tests/unit_test.py

Co-authored-by: Augustin <augustin.lafanechere@gmail.com>

Co-authored-by: Augustin <augustin.lafanechere@gmail.com>

* record extractor interface

* dpath extractor

* docstring

* 🎉 Source File: cache binary stream to file (#15501)

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>

* Docs: update posthog.md (#15541)

* Source Stripe: implement slicing (#15292)

* #45 oncall - source Stripe: implement slicing

* #45 source stripe: upd changelog

* #45 source stripe: upd changelog

* #45 source stripe: make slice range configurable

* #45 source stripe: move generating a single slice into a mixin

* auto-bump connector version [ci skip]

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>

* fix: revert extraEnv delition in values.yaml for bootloader (#15548)

* fix: revert extraEnv delition in values.yaml for bootloader

* add newline

* SAT: backward compatibility - check that cursor fields were not changed (#15520)

* Replace twttr repo to the root build.gradle (#15544)

* Fixed bucket naming for S3

* replaced twttr repo to the root build.gradle

* replaced twttr repo to the root build.gradle

Co-authored-by: Oleksandr Sheheda <alexandr-shegeda@users.noreply.github.com>

* Generate separate server endpoints per domain (#15513)

* 🐛 Backward compatibility test: Don't fail on updating additionalProperties (#15532)

* Source Recurly: adds `state_checkpoint_interval` to streams (#13685)

* Add `state_checkpoint_interval` to Recurly stream

* Bumpg Recurly source version to `0.4.1`

* reset

* use dpath

* enable backward compatibility test

* infer types

* Revert "infer types"

This reverts commit b4de8d6.

* infer some of the types

* some drying

* more drying

* auto-bump connector version [ci skip]

Signed-off-by: Sergey Chvalyuk <grubberr@gmail.com>
Co-authored-by: midavadim <midavadim@yahoo.com>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Co-authored-by: Xiaohan Song <xiaohan@airbyte.io>
Co-authored-by: Liren Tu <tuliren.git@outlook.com>
Co-authored-by: Teal Larson <LARSON.TEAL@GMAIL.COM>
Co-authored-by: Marcos Marx <marcosmarxm@users.noreply.github.com>
Co-authored-by: Ricky Renner <renner@amendllc.com>
Co-authored-by: Davin Chia <davinchia@gmail.com>
Co-authored-by: Brian Lai <51336873+brianjlai@users.noreply.github.com>
Co-authored-by: Augustin <augustin.lafanechere@gmail.com>
Co-authored-by: Serhii Chvaliuk <grubberr@gmail.com>
Co-authored-by: juliatournant <39640564+juliatournant@users.noreply.github.com>
Co-authored-by: Denys Davydov <davydov.den18@gmail.com>
Co-authored-by: Kyryl Skobylko <xpuska513@gmail.com>
Co-authored-by: VitaliiMaltsev <39538064+VitaliiMaltsev@users.noreply.github.com>
Co-authored-by: Oleksandr Sheheda <alexandr-shegeda@users.noreply.github.com>
Co-authored-by: Jonathan Pearlin <jonathan@airbyte.io>
Co-authored-by: Mohamed Magdy <mohamed.magdy@canary.is>
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.

SAT: test STATE backward compatibility (syntactic checks)
2 participants