🎉Source Paypal Transactions: Removed OAuth option

[lazebnyi]

What

#15518 - Source Paypal Transactions: Remove OAuth option

How

Shifted client_id and client_secret fields to top.

Recommended reading order

1. x.java
2. y.python

Pre-merge Checklist

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

[lazebnyi]

/test connector=connectors/source-paypal-transaction

🕑 connectors/source-paypal-transaction https://github.com/airbytehq/airbyte/actions/runs/2878757019
✅ connectors/source-paypal-transaction https://github.com/airbytehq/airbyte/actions/runs/2878757019
Python tests coverage:

Name Stmts Miss Cover ----------------------------------------------------------- source_paypal_transaction/utils.py 8 0 100% source_paypal_transaction/__init__.py 2 0 100% source_paypal_transaction/source.py 272 18 93% ----------------------------------------------------------- TOTAL 282 18 94% Name Stmts Miss Cover Missing ---------------------------------------------------------------------------------- source_acceptance_test/base.py 10 4 60% 15-18 source_acceptance_test/config.py 83 6 93% 78-80, 84-86 source_acceptance_test/conftest.py 164 164 0% 6-282 source_acceptance_test/plugin.py 48 48 0% 6-104 source_acceptance_test/tests/test_core.py 329 111 66% 39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577 source_acceptance_test/tests/test_full_refresh.py 52 2 96% 34, 65 source_acceptance_test/tests/test_incremental.py 121 25 79% 21-23, 29-31, 36-43, 48-61, 208-216 source_acceptance_test/utils/asserts.py 37 2 95% 57-58 source_acceptance_test/utils/common.py 77 17 78% 15-16, 24-30, 47-54, 64, 67 source_acceptance_test/utils/compare.py 62 23 63% 21-51, 68, 97-99 source_acceptance_test/utils/connector_runner.py 110 48 56% 23-26, 32, 36, 39-64, 67-69, 72-74, 77-79, 82-84, 87-89, 92-110, 144-146 source_acceptance_test/utils/json_schema_helper.py 105 13 88% 30-31, 38, 41, 65-68, 96, 120, 190-192 ---------------------------------------------------------------------------------- TOTAL 1321 463 65% 

Build Passed

Test summary info:

=========================== short test summary info ============================ SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestFullRefresh.test_sequential_reads because not found in the config SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:60: Skipping TestIncremental.test_two_sequential_reads because not found in the config ======================== 27 passed, 2 skipped in 34.73s ======================== 

[sherifnada]

is client_id / client_secret authentication the equivalent of username/password? I.e: the user can independently obtain it? the param names make it sound like it's an oauth option.

If yes, then why are we moving this to the top level instead of simply removing the oneOf option? Aren't we going to need to add this back in the future once we enter paypal's partner program? why make 2 backwards breaking changes?

[lazebnyi]

@sherifnada
No, client_id / client_secret is not the equivalent of username/password. More details - here

I think better for UI/UX is leave only client_id / client_secret. We already have logic to support backwards compatibility - here, so I think we won't have problem when we rollback to OAuth support.

[sherifnada]

@lazebnyi so the idea is that in the future we will re-add the refresh_token as needed?

Also, have you manually verified that a connector created with the previous version in the UI will continue to function when you visit the settings page for that connector? e.g: create connector with old versino, upgrade version, go to that connector's setup page, does it continue to work?

[sherifnada]

I guess it seems to me like there is a lot of risks involved in making this backwards breaking change, because it's not just the connector which makes assumptions about the shape. It's the platform too. There is almost no downside to keeping the current structure and just removing one of the oneOf options until we have config migrations. Does it make sense why that's the case?

[lazebnyi]

@sherifnada Yes, for all combination auth will be work:

Old config - {"client_id": "client_id", "secret": "secret"} - Supported

OAuth config - {"credentials": {"auth_method": "oauth2.0", "client_id": "client_id", "client_secret": "client_secret", "refresh_token": "refresh_token" }} - Supported

Private OAuth - {"credentials": {"auth_method": "private_oauth", "client_id": "client_id", "client_secret": "client_secret"}} - Supported

New config - {"client_id": "client_id", "secret": "secret"} or {"client_id": "client_id", "secret": "secret", "refresh_token": "refresh_token"} - Supported

[lazebnyi]

/publish connector=connectors/source-paypal-transaction

🕑 Publishing the following connectors:
connectors/source-paypal-transaction
https://github.com/airbytehq/airbyte/actions/runs/2916626366

Connector	Did it publish?	Were definitions generated?
connectors/source-paypal-transaction	✅	✅

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