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] Remove audience reports hourly streams #20598

Merged
merged 6 commits into from Dec 19, 2022

Conversation

brianjlai
Copy link
Contributor

What

Resolves an issue for certain TikTok streams where we were receiving back:

{'code': 40002, 'message': 'Conflict occurs. The input params including metrics and dimensions have no relation.', 'request_id': '20221215073730DB34302A4354C213D7D7', 'data': {}}

How

For some of our Audience Report streams, TikTok would be throwing an error when we included the stat_time_hour dimension. After going through the TikTok API documentation here. It appears that they have deprecated that dimension. Other time dimensions like Daily (and Lifetime for Advertiser Audience Reports) still work as expected. Given that we no longer can make requests using these dimensions we should remove these streams entirely to prevent future customers from configuring them during setup because they are effectively unusable.

Note: This would affect existing customers, but most likely all customers using these streams would have failing syncs because of it. Customers that did not enable these streams should be unaffected.

@@ -108,13 +108,6 @@
"json_schema": {},
"supported_sync_modes": ["full_refresh"]
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This streams.json I don't think is actually used in our acceptance test YAML but just keeping this up to date regardless

@brianjlai
Copy link
Contributor Author

brianjlai commented Dec 16, 2022

/test connector=connectors/source-tiktok-marketing

🕑 connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/3716059552
❌ connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/3716059552
🐛 https://gradle.com/s/uj74qkfbqwxkk

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED unit_tests/unit_test.py::test_source_streams[secrets/prod_config.json-26]
	 FAILED unit_tests/unit_test.py::test_source_streams[secrets/config.json-19]
	 �[31m================= �[31m�[1m2 failed�[0m, �[32m44 passed�[0m, �[33m172 warnings�[0m�[31m in 20.21s�[0m�[31m ==================�[0m

@octavia-squidington-iv octavia-squidington-iv added the area/documentation Improvements or additions to documentation label Dec 16, 2022
@@ -134,8 +134,8 @@ def test_random_items(prepared_prod_args):
@pytest.mark.parametrize(
"config, stream_len",
[
(PROD_CONFIG_FILE, 26),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have 4 less hourly streams (1 for each audience report) in prod accounts
We have 3 less hourly streams (advertiser audience report is prod only) for sandbox accounts.

@brianjlai
Copy link
Contributor Author

brianjlai commented Dec 16, 2022

/test connector=connectors/source-tiktok-marketing

🕑 connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/3716237302

@brianjlai
Copy link
Contributor Author

brianjlai commented Dec 16, 2022

/test connector=connectors/source-tiktok-marketing

🕑 connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/3716607577

@@ -28,22 +28,22 @@ tests:
discovery:
- config_path: "secrets/prod_config.json"
backward_compatibility_tests_config:
disable_for_version: "0.1.17"
disable_for_version: "1.0.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing streams are considered a breaking change, however we need to do this because the streams aren't usable any more. We will communicate this to the customers attempting to use these non-working streams to reset or disable them.

@brianjlai
Copy link
Contributor Author

brianjlai commented Dec 17, 2022

/test connector=connectors/source-tiktok-marketing

🕑 connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/3721473497
✅ connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/3721473497
Python tests coverage:

Name                                  Stmts   Miss  Cover
---------------------------------------------------------
source_tiktok_marketing/__init__.py       2      0   100%
source_tiktok_marketing/source.py        64      6    91%
source_tiktok_marketing/streams.py      352     37    89%
---------------------------------------------------------
TOTAL                                   418     43    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       140      5    96%   87, 93, 238, 242-243
	 source_acceptance_test/conftest.py                     208     92    56%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-280, 288-301, 306-312, 319-330, 337-353
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1603    336    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
SKIPPED [6] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:386: Backward compatibility tests are disabled for version 1.0.0.
=========== 88 passed, 7 skipped, 31 warnings in 6524.76s (1:48:44) ============

@brianjlai
Copy link
Contributor Author

brianjlai commented Dec 17, 2022

/publish connector=connectors/source-tiktok-marketing

🕑 Publishing the following connectors:
connectors/source-tiktok-marketing
https://github.com/airbytehq/airbyte/actions/runs/3721990452


Connector Did it publish? Were definitions generated?
connectors/source-tiktok-marketing

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@octavia-squidington-iii octavia-squidington-iii temporarily deployed to more-secrets December 18, 2022 00:19 — with GitHub Actions Inactive
@girarda girarda temporarily deployed to more-secrets December 19, 2022 09:07 — with GitHub Actions Inactive
@girarda girarda temporarily deployed to more-secrets December 19, 2022 09:08 — with GitHub Actions Inactive
@girarda
Copy link
Contributor

girarda commented Dec 19, 2022

thank you for taking care of this @brianjlai!

@girarda girarda merged commit 9f0bfa2 into master Dec 19, 2022
@girarda girarda deleted the brian/tiktok_remove_hourly_audience_reports branch December 19, 2022 12:21
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

5 participants