🎉 Tiktok marketing - remove odd auth method #12545

midavadim · 2022-05-03T19:03:58Z

What

Describe what the change is solving
It helps to add screenshots if it affects the frontend.

How

Describe the solution

🚨 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

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

…tion' into midavadim/11029-fix-spec-description

…entication

CLAassistant · 2022-05-03T19:04:04Z

All committers have signed the CLA.

midavadim · 2022-05-03T19:10:22Z

/test connector=connectors/source-tiktok-marketing

🕑 connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/2265826198
❌ connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/2265826198
🐛 https://gradle.com/s/id6j3m5brqb5g
Python short test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestSpec::test_match_expected[inputs0] - AssertionError:...
=================== 1 failed, 56 passed in 484.54s (0:08:04) ===================

…tok-remove-odd-auth-method # Conflicts: # airbyte-integrations/connectors/source-tiktok-marketing/source_tiktok_marketing/spec.py

…d-auth-method' into midavadim/12499-tiktok-remove-odd-auth-method # Conflicts: # airbyte-integrations/connectors/source-tiktok-marketing/source_tiktok_marketing/spec.py

midavadim · 2022-05-03T23:18:42Z

/test connector=connectors/source-tiktok-marketing

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

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  896    259    71%
Name                                  Stmts   Miss  Cover
---------------------------------------------------------
source_tiktok_marketing/spec.py          58      0   100%
source_tiktok_marketing/source.py        49      0   100%
source_tiktok_marketing/__init__.py       2      0   100%
source_tiktok_marketing/streams.py      316     24    92%
---------------------------------------------------------
TOTAL                                   425     24    94%

sherifnada · 2022-05-05T04:21:34Z

airbyte-integrations/connectors/source-tiktok-marketing/integration_tests/spec.json

@@ -5,43 +5,13 @@
    "title": "TikTok Marketing Source Spec",
    "type": "object",
    "properties": {
-      "credentials": {


this will break existing configs, so we shouldn't rename it

All OLD auth_types:
prod_access_token
sandbox_access_token
oauth2.0

1). If I just remove the odd auth method (oauth2.0) then old "OAuth2.0" related config fails validation.

So in order to support all possible old configs I renamed 'credentials' to 'credentials_all'.
It allows successful validation for all possible configs.

2). But if we know that all old "OAuth2.0" config can be updated to new format (and use prod_access_token instead) then 'credentials' can be left unchanged.

@sherifnada what do you think about 2) ? I don't know how we manage "OAuth2.0" in airbyte cloud.

@midavadim my apologies I didn't see the application logic change below. I see now how this was maintaining backwards compatibility.

IMO the simpler thing to do here would be to remove the prod_access_token -- my guess is most people are using oauth or sandbox. if it fails, they'll have to fix it, but renaming the whole object means that everyone using tiktok will have to re-set their configuration in the UI. I think until we have migrations that's a better outcome than having an old branch in the code that lives forever. So I would support removing prod_access_token

midavadim · 2022-05-03T19:32:22Z

airbyte-integrations/connectors/source-tiktok-marketing/source_tiktok_marketing/spec.py

-    credentials: Union[OauthCredSpec, ProductionEnvSpec, SandboxEnvSpec] = Field(
-        title="Authentication *", order=0, default={}, type="object"
-    )
+    credentials_all: Union[ProductionEnvSpec, SandboxEnvSpec] = Field(title="Authentication *", order=0, default={}, type="object")


rename 'credentials' to 'credentials_all' is order to support new and old configs (with oauth2.0) otherwise old configs fails validation

midavadim · 2022-05-05T17:50:55Z

airbyte-integrations/connectors/source-tiktok-marketing/integration_tests/spec.json

@@ -5,43 +5,13 @@
    "title": "TikTok Marketing Source Spec",
    "type": "object",
    "properties": {
-      "credentials": {


All OLD auth_types:
prod_access_token
sandbox_access_token
oauth2.0

1). If I just remove the odd auth method (oauth2.0) then old "OAuth2.0" related config fails validation.

So in order to support all possible old configs I renamed 'credentials' to 'credentials_all'.
It allows successful validation for all possible configs.

2). But if we know that all old "OAuth2.0" config can be updated to new format (and use prod_access_token instead) then 'credentials' can be left unchanged.

@sherifnada what do you think about 2) ? I don't know how we manage "OAuth2.0" in airbyte cloud.

midavadim · 2022-05-06T20:32:48Z

/test connector=connectors/source-tiktok-marketing

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

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        74      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/tests/test_core.py              285    106    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
source_acceptance_test/tests/test_incremental.py        69     38    45%
------------------------------------------------------------------------
TOTAL                                                  896    259    71%
Name                                  Stmts   Miss  Cover
---------------------------------------------------------
source_tiktok_marketing/spec.py          58      0   100%
source_tiktok_marketing/source.py        49      0   100%
source_tiktok_marketing/__init__.py       2      0   100%
source_tiktok_marketing/streams.py      316     24    92%
---------------------------------------------------------
TOTAL                                   425     24    94%

…tok-remove-odd-auth-method # Conflicts: # airbyte-integrations/connectors/source-tiktok-marketing/integration_tests/spec.json # airbyte-integrations/connectors/source-tiktok-marketing/source_tiktok_marketing/spec.py

midavadim · 2022-05-06T21:42:31Z

/publish connector=connectors/source-tiktok-marketing

🕑 connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/2284175679
🚀 Successfully published connectors/source-tiktok-marketing
🚀 Auto-bumped version for connectors/source-tiktok-marketing
✅ connectors/source-tiktok-marketing https://github.com/airbytehq/airbyte/actions/runs/2284175679

…d-auth-method' into midavadim/12499-tiktok-remove-odd-auth-method

sherifnada · 2022-05-06T22:22:54Z

@midavadim shouldn't this be merged? :)

midavadim · 2022-05-06T22:25:43Z

@sherifnada it was merged 10 mins before your comment :)

sherifnada · 2022-05-06T22:26:12Z

ah sorry I am seeing it as "closed" not merged that's all i meant

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com> Co-authored-by: ievgeniit <etsybaev@gmail.com>

midavadim and others added 8 commits April 28, 2022 17:14

updated seed files

19bb07b

updated change history

6b8bda2

updated docs

288116b

updated connector version

81a9fe1

auto-bump connector version

f4c78e8

updated source specs

61623e7

Merge remote-tracking branch 'origin/midavadim/11029-fix-spec-descrip…

19a053d

…tion' into midavadim/11029-fix-spec-description

removed odd 'oauth2' method which duplicates logic of production auth…

d25af3a

…entication

github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels May 3, 2022

Merge branch 'master' into midavadim/12499-tiktok-remove-odd-auth-method

63a1c08

github-actions bot removed the area/documentation Improvements or additions to documentation label May 3, 2022

midavadim added 3 commits May 3, 2022 22:25

Merge remote-tracking branch 'origin/master' into midavadim/12499-tik…

8ea3855

…tok-remove-odd-auth-method # Conflicts: # airbyte-integrations/connectors/source-tiktok-marketing/source_tiktok_marketing/spec.py

Merge remote-tracking branch 'origin/midavadim/12499-tiktok-remove-od…

a17f0f1

…d-auth-method' into midavadim/12499-tiktok-remove-odd-auth-method # Conflicts: # airbyte-integrations/connectors/source-tiktok-marketing/source_tiktok_marketing/spec.py

updated test spec file

f1e77b5

midavadim requested a review from annalvova05 May 3, 2022 23:21

annalvova05 approved these changes May 4, 2022

View reviewed changes

Added java test for java part of oauth2

9e74aff

midavadim temporarily deployed to more-secrets May 4, 2022 10:16 Inactive

sherifnada suggested changes May 5, 2022

View reviewed changes

Added java tests + fix for TikTik OAuth part

d551a3c

etsybaev temporarily deployed to more-secrets May 5, 2022 11:28 Inactive

midavadim changed the title ~~🎉 Tiktok markeing - remove odd auth method~~ 🎉 Tiktok marketing - remove odd auth method May 5, 2022

midavadim commented May 5, 2022

View reviewed changes

midavadim requested a review from sherifnada May 5, 2022 17:54

midavadim temporarily deployed to more-secrets May 6, 2022 13:05 Inactive

updated ACCESS_TOKEN_URL, the old one caused redirect

e60021d

midavadim temporarily deployed to more-secrets May 6, 2022 20:31 Inactive

midavadim added 2 commits May 7, 2022 00:00

updated connector version

e7d9dad

github-actions bot added the area/documentation Improvements or additions to documentation label May 6, 2022

midavadim temporarily deployed to more-secrets May 6, 2022 21:37 Inactive

auto-bump connector version

2341434

octavia-squidington-iii temporarily deployed to more-secrets May 6, 2022 22:03 Inactive

midavadim added 2 commits May 7, 2022 01:07

updated source_specs.yaml

aa57790

Merge remote-tracking branch 'origin/midavadim/12499-tiktok-remove-od…

bc25177

…d-auth-method' into midavadim/12499-tiktok-remove-odd-auth-method

midavadim temporarily deployed to more-secrets May 6, 2022 22:10 Inactive

midavadim closed this May 6, 2022

This was linked to issues May 6, 2022

[EPIC] Source TikTok to Cloud Beta #11029

Closed

Source tiktok ads: Consolidate the Oauth2.0 and Production Access Token auth types #12499

Closed

sherifnada reopened this May 6, 2022

sherifnada merged commit 22ecd28 into master May 6, 2022

sherifnada deleted the midavadim/12499-tiktok-remove-odd-auth-method branch May 6, 2022 23:48

octavia-squidington-iii mentioned this pull request May 9, 2022

Bump Airbyte version from 0.37.0-alpha to 0.37.1-alpha #12703

Merged

suhomud pushed a commit that referenced this pull request May 23, 2022

🎉 Tiktok marketing - remove odd auth method (#12545)

ea349c5

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com> Co-authored-by: ievgeniit <etsybaev@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉 Tiktok marketing - remove odd auth method #12545

🎉 Tiktok marketing - remove odd auth method #12545

midavadim commented May 3, 2022

CLAassistant commented May 3, 2022 •

edited

Loading

midavadim commented May 3, 2022 •

edited by github-actions bot

Loading

midavadim commented May 3, 2022 •

edited by github-actions bot

Loading

sherifnada May 5, 2022

midavadim May 5, 2022 •

edited

Loading

sherifnada May 5, 2022

midavadim May 3, 2022

midavadim May 5, 2022 •

edited

Loading

midavadim commented May 6, 2022 •

edited by github-actions bot

Loading

midavadim commented May 6, 2022 •

edited by github-actions bot

Loading

sherifnada commented May 6, 2022

midavadim commented May 6, 2022

sherifnada commented May 6, 2022

🎉 Tiktok marketing - remove odd auth method #12545

🎉 Tiktok marketing - remove odd auth method #12545

Conversation

midavadim commented May 3, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

CLAassistant commented May 3, 2022 • edited Loading

midavadim commented May 3, 2022 • edited by github-actions bot Loading

midavadim commented May 3, 2022 • edited by github-actions bot Loading

sherifnada May 5, 2022

Choose a reason for hiding this comment

midavadim May 5, 2022 • edited Loading

Choose a reason for hiding this comment

sherifnada May 5, 2022

Choose a reason for hiding this comment

midavadim May 3, 2022

Choose a reason for hiding this comment

midavadim May 5, 2022 • edited Loading

Choose a reason for hiding this comment

midavadim commented May 6, 2022 • edited by github-actions bot Loading

midavadim commented May 6, 2022 • edited by github-actions bot Loading

sherifnada commented May 6, 2022

midavadim commented May 6, 2022

sherifnada commented May 6, 2022

CLAassistant commented May 3, 2022 •

edited

Loading

midavadim commented May 3, 2022 •

edited by github-actions bot

Loading

midavadim commented May 3, 2022 •

edited by github-actions bot

Loading

midavadim May 5, 2022 •

edited

Loading

midavadim May 5, 2022 •

edited

Loading

midavadim commented May 6, 2022 •

edited by github-actions bot

Loading

midavadim commented May 6, 2022 •

edited by github-actions bot

Loading