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 Tik Tok Marketing: Migration to Low-Code #38316

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

darynaishchenko
Copy link
Collaborator

@darynaishchenko darynaishchenko commented May 17, 2024

What

resolved: https://github.com/airbytehq/airbyte-internal-issues/issues/7824

How

Migrated source to use low-code cdk instead of python cdk.
Main changes:

  • State: Previously all incremental streams used incorrect state without partition. On low-code cdk all incremental streams use per partition state.
  • Lifetime reports: Previously implementation used lifetime=true as request param, which is deprecated on API v1.3. Now lifetime reports use query_lifetime=true, with this param start_date and end_date should not be provided. Exception: advertiser_lifetime_report: API v1.3 doesn't allow query_lifetime=true` with advertiser reports, so this stream was implemented exactly as in py version with start_date and end_date query params(range >=365d)
  • Advertiser Ids stream: schema was changed to use advertiser_id as type of stream to be up to date with API docs.
  • Discover for configs with granularity: In py implementation were missing streams(campaigns_audience_reports, ad_group_audience_reports_by_platform, ad_group_audience_reports_by_country, ads_audience_reports_by_country, advertisers_audience_reports_by_country, campaigns_audience_reports_by_platform, advertisers_audience_reports_by_platform, ads_audience_reports_by_platform, ads_audience_reports_by_province), which users with provided granularity actually can use but streams method didn't return them. For configs with granularity source removes granularity from stream name as it was previously named.

Review guide

User Impact

Breaking change users will need to follow migration guide for affected streams.

Can this PR be safely reverted and rolled back?

Breaking change due to changes in schema and state format.

  • YES 💚
  • NO ❌

@darynaishchenko darynaishchenko self-assigned this May 17, 2024
Copy link

vercel bot commented May 17, 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 May 29, 2024 10:26am

@darynaishchenko darynaishchenko changed the title Source Tik Tok Marketing: Migration to Low-Code 🚨 🚨 ✨ Source Tik Tok Marketing: Migration to Low-Code May 23, 2024
@darynaishchenko darynaishchenko marked this pull request as ready for review May 23, 2024 17:34
@octavia-squidington-iv octavia-squidington-iv requested a review from a team May 23, 2024 17:36
@darynaishchenko darynaishchenko requested a review from a team May 24, 2024 15:46
else:
stream_state = stream_states[0]
kwargs = {"stream_state": stream_state, "stream_slice": stream_slice, "next_page_token": next_page_token}
return [record for record in records if self._filter_interpolator.eval(self.config, record=record, **kwargs)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please update to align with original signature (Iterable)


requester:
type: HttpRequester
url_base: '{{ "https://sandbox-ads.tiktok.com/open_api/v1.3/" if config.get(''credentials'', {})[''auth_type''] == "sandbox_access_token" else "https://business-api.tiktok.com/open_api/v1.3/" }}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
url_base: '{{ "https://sandbox-ads.tiktok.com/open_api/v1.3/" if config.get(''credentials'', {})[''auth_type''] == "sandbox_access_token" else "https://business-api.tiktok.com/open_api/v1.3/" }}'
url_base: '"https://{{ "sandbox-ads" if config.get(''credentials'', {})[''auth_type''] == "sandbox_access_token" else "business-api" }}.tiktok.com/open_api/v1.3/"'

nit

type: DpathExtractor
field_path: ["data", "list"]

record_selector_with_filter_by_midify_time:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
record_selector_with_filter_by_midify_time:
record_selector_with_filter_by_modify_time:

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.

None yet

3 participants