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

Add currently syncing #24

Merged
merged 36 commits into from
Mar 14, 2022
Merged

Add currently syncing #24

merged 36 commits into from
Mar 14, 2022

Conversation

dsprayberry
Copy link
Contributor

@dsprayberry dsprayberry commented Mar 3, 2022

Description of change

  • Implements proper use of currently_syncing.
  • Adds customerId to state for each stream.
  • Reorders sync hierarchy to iterate over each customer for each stream.

QA steps

  • automated tests passing
  • manual qa steps passing (list below)
    • Tests passing locally.
    • Added unit tests for currently_syncing shuffle

Risks

Minimal; tap is in alpha.

Rollback steps

  • revert this branch

tap_google_ads/streams.py Outdated Show resolved Hide resolved
tap_google_ads/streams.py Outdated Show resolved Hide resolved
tap_google_ads/sync.py Outdated Show resolved Hide resolved
tap_google_ads/sync.py Outdated Show resolved Hide resolved
tap_google_ads/sync.py Show resolved Hide resolved
dsprayberry and others added 5 commits March 11, 2022 16:26
* Add optional end date, add unit tests

Co-authored-by: Andy Lu <andy@stitchdata.com>

* Test functions can't be named run_test apparently

* Rename do_thing

* Extract `get_queries_from_sync` as a function

* Remove unused variable

* Refactor tests to be more explicit

* Mock singer.utils.now to return a specific date

Co-authored-by: Andy Lu <andy@stitchdata.com>
@luandy64 luandy64 merged commit 3c1357c into main Mar 14, 2022
@luandy64 luandy64 deleted the add-currently-syncing branch March 14, 2022 17:47
kspeer825 pushed a commit that referenced this pull request Mar 15, 2022
* Sync all customers for a given stream

* Add logging to see when we retry requests

* Update currently_syncing with customerId too. Write state as soon as we
update it

* Add the customerId to the bookmark keys

* Add shuffle for customerId and tap_stream_id; add shuffle unit tests

* Bug fix for when currently_syncing is null

* Fix exception handling typeError

* Fix none cases for currently_syncing

* Fix currently_syncing to write a tuple we can read in later

* Add get_customer_ids so we can use it in the tests

* Fix manipulated_state to account for customer_ids

* Update assertion for currently_syncing

* Fix currently syncing assertion

* Move bookmark access into Full Table assertions section

Full Table doesn't need the "stream_name and customer id" key logic

* Remove duplicate assertion

* Revert 6db016e

* Update bookmark to read stream->customer->replication_key

* Update tap to write bookmarks as stream->customer->replication_key

* Update manipulated state to nest stream->customer->replication_key

* Run bookmark assertions for every customer

* Fix dict comprehension typo

* Fix conflict with main

* Remove `get_state_key` again, use env var instead of hardcoded value

* Add missing dependency

* Move currently-syncing-null-out to the end of sync to prevent gaps

* Sort selected_streams and customers to guarantee consistency across runs

* Don't let the tap write (None, None)

* Sort selected_streams and customers effectively

* Update currently_syncing test assertions

* Add sort functions for streams and customers

* Update `shuffle` to handle a missing value

* Update unit tests to use sort_function, add a test for shuffling streams

* Add end date (#28)

* Add optional end date, add unit tests

Co-authored-by: Andy Lu <andy@stitchdata.com>

* Test functions can't be named run_test apparently

* Rename do_thing

* Extract `get_queries_from_sync` as a function

* Remove unused variable

* Refactor tests to be more explicit

* Mock singer.utils.now to return a specific date

Co-authored-by: Andy Lu <andy@stitchdata.com>

* add conversion_window test

* fixed conversion window unittests, bug removed

Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com>
Co-authored-by: Andy Lu <andy@stitchdata.com>
Co-authored-by: kspeer <kspeer@stitchdata.com>
luandy64 added a commit that referenced this pull request Mar 16, 2022
* add conversion window test

* add conversion window test

* wip updated tests to worka with currently syncing dev branch [skip ci]

* Revert removal of metric compatibility removal (#29)

* Revert removal of metric compatibility removal

* Whitespace cleanup

* Add currently syncing (#24)

* Sync all customers for a given stream

* Add logging to see when we retry requests

* Update currently_syncing with customerId too. Write state as soon as we
update it

* Add the customerId to the bookmark keys

* Add shuffle for customerId and tap_stream_id; add shuffle unit tests

* Bug fix for when currently_syncing is null

* Fix exception handling typeError

* Fix none cases for currently_syncing

* Fix currently_syncing to write a tuple we can read in later

* Add get_customer_ids so we can use it in the tests

* Fix manipulated_state to account for customer_ids

* Update assertion for currently_syncing

* Fix currently syncing assertion

* Move bookmark access into Full Table assertions section

Full Table doesn't need the "stream_name and customer id" key logic

* Remove duplicate assertion

* Revert 6db016e

* Update bookmark to read stream->customer->replication_key

* Update tap to write bookmarks as stream->customer->replication_key

* Update manipulated state to nest stream->customer->replication_key

* Run bookmark assertions for every customer

* Fix dict comprehension typo

* Fix conflict with main

* Remove `get_state_key` again, use env var instead of hardcoded value

* Add missing dependency

* Move currently-syncing-null-out to the end of sync to prevent gaps

* Sort selected_streams and customers to guarantee consistency across runs

* Don't let the tap write (None, None)

* Sort selected_streams and customers effectively

* Update currently_syncing test assertions

* Add sort functions for streams and customers

* Update `shuffle` to handle a missing value

* Update unit tests to use sort_function, add a test for shuffling streams

* Add end date (#28)

* Add optional end date, add unit tests

Co-authored-by: Andy Lu <andy@stitchdata.com>

* Test functions can't be named run_test apparently

* Rename do_thing

* Extract `get_queries_from_sync` as a function

* Remove unused variable

* Refactor tests to be more explicit

* Mock singer.utils.now to return a specific date

Co-authored-by: Andy Lu <andy@stitchdata.com>

* add conversion_window test

* fixed conversion window unittests, bug removed

Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com>
Co-authored-by: Andy Lu <andy@stitchdata.com>
Co-authored-by: kspeer <kspeer@stitchdata.com>

* Bump to v0.2.0, update changelog (#31)

* Bump to v0.2.0, update changelog

* Add link for this PR, fix link syntax

* Update changelog format

* expanded conversion window testing for error case, BUG linked

* parallelism 8 -> 12

* added unittest for start date within conversion window

Co-authored-by: kspeer <kspeer@stitchdata.com>
Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com>
Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com>
Co-authored-by: Andy Lu <andy@stitchdata.com>
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.

None yet

3 participants