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

✨ Source Tiktok Marketing: Support concurrency #51584

Merged
merged 17 commits into from
Jan 31, 2025

Conversation

maxi297
Copy link
Contributor

@maxi297 maxi297 commented Jan 16, 2025

What

Updated the Airbyte CDK to version 6, which introduces concurrency.

How

  • Updated the Airbyte CDK version.
  • Deprecated the record filter for some streams and replaced it with client_side_incremental.

The following streams are now concurrent:

- advertiser_ids
- advertisers
- audiences
- creative_assets_music
- creative_assets_portfolios
- campaigns
- ad_groups
- ads
- creative_assets_images
- creative_assets_videos
- ads_reports_lifetime
- advertisers_reports_lifetime
- campaigns_reports_lifetime
- ad_groups_reports_lifetime
- advertisers_audience_reports_lifetime

User Impact

Syncs should run faster due to improved concurrency settings.

⚠️ Breaking Change for users with report_granularity:

  • The report_granularity option has been deprecated.
  • Affected streams will now include a granularity suffix (*_hourly, *_daily, *_lifetime).

Affected Streams (for users with report_granularity):

- ads_reports
- ad_groups_reports
- advertisers_reports
- campaigns_reports
- campaigns_audience_reports
- ad_group_audience_reports
- ads_audience_reports
- advertisers_audience_reports
- campaigns_audience_reports_by_country
- ad_group_audience_reports_by_country
- ads_audience_reports_by_country
- advertisers_audience_reports_by_country
- campaigns_audience_reports_by_platform
- ad_group_audience_reports_by_platform
- ads_audience_reports_by_platform
- advertisers_audience_reports_by_platform
- ads_audience_reports_by_province

Users must reset their source configuration, refresh the schema, and clear data for the impacted streams after upgrading.

Can this PR be safely reverted and rolled back?

  • YES 💚 (Not safe to roll back if the report_granularity option is present in the config)
  • NO ❌

Copy link

vercel bot commented Jan 16, 2025

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

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2025 6:02pm

@natikgadzhi
Copy link
Contributor

@tolik0 wrapping this up this week, or are we overflowing into q1?

@tolik0
Copy link
Contributor

tolik0 commented Jan 28, 2025

@natikgadzhi Yes, I will merge this PR this week

@tolik0
Copy link
Contributor

tolik0 commented Jan 29, 2025

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (c9d05a4)

@maxi297
Copy link
Contributor Author

maxi297 commented Jan 29, 2025

/autofix

EDIT: I tried :(

@tolik0
Copy link
Contributor

tolik0 commented Jan 29, 2025

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (47cd1d2)

@brianjlai
Copy link
Contributor

brianjlai commented Jan 30, 2025

Breaking Change for users with report_granularity:
The report_granularity option has been deprecated.
Affected streams will now include a granularity suffix (*_hourly, *_daily, *_lifetime).

Was this not already the case? Looking at the metabase dashboard, we have a good number of ads_reports_daily among other types of report granular prefixed names. So wasn't sure if this description was just not up to date

Users must reset their source configuration, refresh the schema, and clear data for the impacted streams after upgrading.

Are we going to coordinate this release w/ support so they are aware of things? Also, I thought the progressive rollout release process was not suitable for "Progressive rollouts for user-facing changes (which require an opt-in) are not supported yet"

@tolik0
Copy link
Contributor

tolik0 commented Jan 30, 2025

@brianjlai We don’t have any users in the Cloud who have this option in their config. This parameter was not included in the spec, but we were still renaming the streams, which led to an error.

@tolik0 tolik0 merged commit e91b74d into master Jan 31, 2025
29 checks passed
@tolik0 tolik0 deleted the maxi297/tiktok-marketing-concurrent branch January 31, 2025 11:01
@brianjlai
Copy link
Contributor

@brianjlai We don’t have any users in the Cloud who have this option in their config. This parameter was not included in the spec, but we were still renaming the streams, which led to an error.

understood. thanks for the explanation. no other issues for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/tiktok-marketing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants