Skip to content

Sync record from start date for streams - campaigns, ad groups, ads #22

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

Merged
merged 6 commits into from
Jun 22, 2023

Conversation

sgandhi1311
Copy link
Member

@sgandhi1311 sgandhi1311 commented Jun 16, 2023

Description of change

Initially, records older than the start date were included in the target DB. To refine this, Looked at the API documentation. We cannot query via an updated_at or modified_at, only by create_time so we will be treating the stream as pseudo-incremental, aka full table but we only emit records with a replication key greater than our bookmark

Manual QA steps

  • Integration test for the start date is passing with all the conditions.
  • Checked sync locally.

Risks

  • low

Rollback steps

  • revert this branch

Comment on lines 325 to 329
for date_batch in date_batches:
filter = {'create_start_time': strftime(date_batch['start_date'], FILTER_DATE_FORMAT),
'create_end_time': strftime(date_batch['end_date'], FILTER_DATE_FORMAT)}
self.params['filtering'] = json.dumps(filter)
self.sync_pages(stream)

Choose a reason for hiding this comment

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

Let's say we are extracting campaigns stream and We start date is 2022-04-21 then we will miss such records.

id create_time modify_time
1 2022-04-18T08:32:17 2022-04-21T08:32:17

Please re-verify the logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed those filters. As the API does not support querying using the replication key, therefore, pseudo-incremental (aka full table) logic is implemented. Now, Tap will only emit records with a replication key greater than our bookmark

@sgandhi1311 sgandhi1311 requested a review from RushiT0122 June 21, 2023 08:59
Copy link

@RushiT0122 RushiT0122 left a comment

Choose a reason for hiding this comment

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

Approved.

Remove leading space on L#194.

@sgandhi1311 sgandhi1311 merged commit 8ad0d87 into master Jun 22, 2023
LesleyDing pushed a commit to vibeus/tap-tiktok-ads that referenced this pull request Mar 25, 2024
…inger-io#22)

* Sync record from start date for streams - campaigns, ad groups, ads

* fix start date test

* remove the query filter and do pseudo increment for streams

* fix the unit test cases

* remove unused code

* setup and changelog update

---------

Co-authored-by: RushiT0122 <rtodkar@stitchdata-talend.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.

2 participants