-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 Titok marketing: update schema. add unittest, update SAT, fix initialization for Audience streams #11378
Conversation
…, basic_resports, audience_reports) with DAY and LIFETIME granularities. Advertizer ids are extracted only during read method, previously ids were extracted during stream initialization. Added caching for AdvertizerIds stream
- config_path: "secrets/prod_config_day.json" | ||
configured_catalog_path: "integration_tests/streams_all.json" | ||
timeout_seconds: 1200 | ||
validate_schema: false # validation fails for audience report streams |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we aware of what exactly is wrong with the validation of Audience Report
streams?
"cash_spend": { | ||
"type": ["null", "number"] | ||
}, | ||
"voucher_spend": { | ||
"type": ["null", "number"] | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any valid reason we removed this from the schema?
@@ -108,6 +114,14 @@ class TiktokStream(HttpStream, ABC): | |||
# max value of page | |||
page_size = 1000 | |||
|
|||
def __init__(self, **kwargs): | |||
super().__init__(authenticator=kwargs.get("authenticator")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to call authenticator
directly here? I believe it should be passed automatically if we have such a property inside the passing args. Please check.
/test connector=connectors/source-tiktok-marketing
|
/test connector=connectors/source-tiktok-marketing
|
/test connector=connectors/source-tiktok-marketing
|
/test connector=connectors/source-tiktok-marketing
|
/test connector=connectors/source-tiktok-marketing
|
/test connector=connectors/source-tiktok-marketing
|
/publish connector=connectors/source-tiktok-marketing
|
…o midavadim/tiktok-schema-fix
…hema-fix # Conflicts: # airbyte-config/init/src/main/resources/seed/source_specs.yaml
…x initialization for Audience streams (#11378) * update schemas * Added tests (basixm full_refresh, incremental) for all streams (basic, basic_resports, audience_reports) with DAY and LIFETIME granularities. Advertizer ids are extracted only during read method, previously ids were extracted during stream initialization. Added caching for AdvertizerIds stream * added description for _advertiser_ids usage and * applied format * fixed unit test * fixed get_advertiser_ids() usage * added unit test * removed enum attr for const properties * split audience report incremental tests * updated docs * updated docs and updated tests * added conditions for sandbox env, updated docs and updated tests * update connector version to 0.1.6 * auto-bump connector version * update source_specs.yaml * update source_specs.yaml Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Engineering Acceptance Criteria:
Source Tiktok Marketing: have at least 90% unit test coverage #11372
Source Tiktok Marketing: add missing streams in SAT #11376
Source Tiktok marketing: add SAT tests for DAY, HOUR granularity #11377
Outstanding bugs:
Source Tiktok: data type error for mobile_app_id field in ads_reports_metrics and ad_groups_reports_metrics #9709
Source Tiktok: wrong type in campaign schema #9491
Round value of source attribute with type "integer" #6875
Source Tiktok: fix metrix for LIFETIME granularity #11375
Source Titok marketing: discovery method is too long (~ 40 seconds) #12107
Recommended reading order
x.java
y.python
🚨 User Impact 🚨
Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changesTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.