🐛 source-paypal-transaction: Keep transaction_id field as a string and shorten step to 7 days #31759

girarda · 2023-10-23T23:45:21Z

What

Fixes two issues we ran into with the paypal-transaction last week

the transaction_id field was converted to a number. It is now kept as as a string by setting the value_type field in the manifest.yaml
Shorten the size of the time window from 15 days to 7 days to avoid requesting too much data at once. The original Python connector had logic to split the stream_slice in two if the API said it was requesting too much data. This feature is not available in the low-code framework, but preemptively splitting the slice should alleviate the issue.

Both issues had to be resolved to get the broken connection to succeed with the new connector.

🚨 User Impact 🚨

This is not a breaking change from 2.1.1, but it is a breaking change from 2.0.0
Full refresh syncs will potentially take more time because the connector is fetching data in smaller batches.

Pre-merge Actions

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Community member? Grant edit access to maintainers (instructions)
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.
Connector version is set to 0.0.1
- Dockerfile has version 0.0.1
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- docs/integrations/<source or destination>/<name>.md including changelog with an entry for the initial version. See changelog example
- docs/integrations/README.md

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.

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Unit & integration tests added

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.

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:generateScaffolds then checking in your changes
Documentation which references the generator is updated as needed

Updating the Python CDK

Airbyter

Before merging:

Pull Request description explains what problem it is solving
Code change is unit tested
Build and my-py check pass
Smoke test the change on at least one affected connector
- On Github: Run this workflow, passing --use-local-cdk --name=source-<connector> as options
- Locally: airbyte-ci connectors --use-local-cdk --name=source-<connector> test
PR is reviewed and approved

After merging:

Publish the CDK
- The CDK does not follow proper semantic versioning. Choose minor if this the change has significant user impact or is a breaking change. Choose patch otherwise.
- Write a thoughtful changelog message so we know what was updated.
Merge the platform PR that was auto-created for updating the Connector Builder's CDK version
- This step is optional if the change does not affect the connector builder or declarative connectors.

vercel · 2023-10-23T23:45:27Z

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		Oct 24, 2023 10:55pm

github-actions · 2023-10-23T23:45:41Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

airbyte-oss-build-runner · 2023-10-23T23:56:56Z

source-paypal-transaction test report (commit `e65495f5ed`) - ✅

⏲️ Total pipeline duration: 03mn12s

Step	Result
Build source-paypal-transaction docker image for platform(s) linux/amd64	✅
Acceptance tests	✅
Check our base image is used	✅
Code format checks	✅
Validate metadata for source-paypal-transaction	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-paypal-transaction test

girarda · 2023-10-24T00:27:06Z

airbyte-integrations/connectors/source-paypal-transaction/setup.py

@@ -6,7 +6,7 @@
 from setuptools import find_packages, setup

 MAIN_REQUIREMENTS = [
-    "airbyte-cdk~=0.1",
+    "airbyte-cdk>=0.51.44",


the value_type field requires 0.51.44

girarda · 2023-10-24T00:30:24Z

docs/integrations/sources/paypal-transaction.md

@@ -87,8 +87,9 @@ Transactions sync is performed with default `stream_slice_period` = 1 day, it me

 | Version | Date       | Pull Request                                             | Subject                                                                                                                      |
 |:--------|:-----------|:---------------------------------------------------------|:-----------------------------------------------------------------------------------------------------------------------------|
-| 2.1.1 | 2023-10-19 | [31599](https://github.com/airbytehq/airbyte/pull/31599) | Base image migration: remove Dockerfile and use the python-connector-base image |
-| 2.1.0   | 2023-08-14 | [29223](https://github.com/airbytehq/airbyte/pull/29223) | Migrate Python CDK to Low Code schema                                                                                                     |
+| 2.1.2 | 2023-10-23 | [31759](https://github.com/airbytehq/airbyte/pull/31759) | Keep transaction_id as a string and fetch data in 7-day batches                                                              |


this is not a breaking change from 2.1.0, but this is a breaking change from 2.0.0. Considering 2.1.0 is broken, is it worth bumping to 3.0.0?

we'll also need to update the connection state on cloud because it's pinned to 2.0.0

Considering 2.1.0 is broken, is it worth bumping to 3.0.0?

I'm not sure. I think it would be more fitting to retrospectively mark 2.1.0 as the breaking change 🤔 Then OSS users upgrading to either that version or to the most recent version directly still are aware that the state change is happening. Cloud users we can take care of.

To be clear, the change to state makes it breaking in the "can't roll back" sense, but the roll forward went okay, right?

ah right! the roll forward was fine so no big deal here 👍

retrospectively mark 2.1.0 as the breaking change
Is this just a matter of updating the metadata file with a breaking change block?

Is this just a matter of updating the metadata file with a breaking change block?

It is!

In this case I believe the cloud users would see the banner, since they're currently back on 2.0.0, but I think we can opt in for them? 🤔 Alternatively we could wait until after this PR to mark 2.1.0 as breaking

ok I can mark 2.1.0 as breaking in a follow up PR. I see the deadline is a required field, but would it be valid to set a date in the past?

yep, that's fine!

airbyte-oss-build-runner · 2023-10-24T00:36:55Z

source-paypal-transaction test report (commit `c55c69c747`) - ✅

⏲️ Total pipeline duration: 02mn39s

Step	Result
Build source-paypal-transaction docker image for platform(s) linux/amd64	✅
Acceptance tests	✅
Check our base image is used	✅
Code format checks	✅
Validate metadata for source-paypal-transaction	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-paypal-transaction test

airbyte-oss-build-runner · 2023-10-24T22:59:27Z

source-paypal-transaction test report (commit `1f4f418f0a`) - ✅

⏲️ Total pipeline duration: 02mn37s

Step	Result
Build source-paypal-transaction docker image for platform(s) linux/amd64	✅
Acceptance tests	✅
Check our base image is used	✅
Code format checks	✅
Validate metadata for source-paypal-transaction	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-paypal-transaction test

update

4f2b346

octavia-squidington-iii added area/connectors Connector related issues connectors/source/paypal-transaction labels Oct 23, 2023

update metadata and changelog

3572335

octavia-squidington-iv requested a review from a team October 23, 2023 23:46

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Oct 23, 2023

vercel bot deployed to Preview October 23, 2023 23:48 View deployment

Bump minimum cdk version

e65495f

girarda commented Oct 24, 2023

View reviewed changes

Merge branch 'master' into alex/fix_paypal_transaction

c55c69c

girarda requested a review from erohmensing October 24, 2023 00:32

vercel bot deployed to Preview October 24, 2023 00:34 View deployment

bazarnov approved these changes Oct 24, 2023

View reviewed changes

girarda added checklist-action-run and removed checklist-action-run labels Oct 24, 2023

remove cloud pin

1f4f418

girarda merged commit 23e1db0 into master Oct 25, 2023
19 checks passed

girarda deleted the alex/fix_paypal_transaction branch October 25, 2023 03:02

girarda mentioned this pull request Oct 25, 2023

🐛 source-paypal-transaction: retroactively mark 2.1.0 as a breaking change #31806

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 source-paypal-transaction: Keep transaction_id field as a string and shorten step to 7 days #31759

🐛 source-paypal-transaction: Keep transaction_id field as a string and shorten step to 7 days #31759

girarda commented Oct 23, 2023 •

edited

Loading

vercel bot commented Oct 23, 2023 •

edited

Loading

github-actions bot commented Oct 23, 2023 •

edited by girarda

Loading

airbyte-oss-build-runner commented Oct 23, 2023

girarda Oct 24, 2023

girarda Oct 24, 2023

girarda Oct 24, 2023

erohmensing Oct 24, 2023 •

edited

Loading

girarda Oct 24, 2023

erohmensing Oct 24, 2023

girarda Oct 24, 2023

erohmensing Oct 25, 2023

airbyte-oss-build-runner commented Oct 24, 2023

airbyte-oss-build-runner commented Oct 24, 2023

🐛 source-paypal-transaction: Keep transaction_id field as a string and shorten step to 7 days #31759

🐛 source-paypal-transaction: Keep transaction_id field as a string and shorten step to 7 days #31759

Conversation

girarda commented Oct 23, 2023 • edited Loading

What

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Airbyter

vercel bot commented Oct 23, 2023 • edited Loading

github-actions bot commented Oct 23, 2023 • edited by girarda Loading

Before Merging a Connector Pull Request

airbyte-oss-build-runner commented Oct 23, 2023

source-paypal-transaction test report (commit e65495f5ed) - ✅

girarda Oct 24, 2023

Choose a reason for hiding this comment

girarda Oct 24, 2023

Choose a reason for hiding this comment

girarda Oct 24, 2023

Choose a reason for hiding this comment

erohmensing Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

girarda Oct 24, 2023

Choose a reason for hiding this comment

erohmensing Oct 24, 2023

Choose a reason for hiding this comment

girarda Oct 24, 2023

Choose a reason for hiding this comment

erohmensing Oct 25, 2023

Choose a reason for hiding this comment

airbyte-oss-build-runner commented Oct 24, 2023

source-paypal-transaction test report (commit c55c69c747) - ✅

airbyte-oss-build-runner commented Oct 24, 2023

source-paypal-transaction test report (commit 1f4f418f0a) - ✅

girarda commented Oct 23, 2023 •

edited

Loading

vercel bot commented Oct 23, 2023 •

edited

Loading

github-actions bot commented Oct 23, 2023 •

edited by girarda

Loading

source-paypal-transaction test report (commit `e65495f5ed`) - ✅

erohmensing Oct 24, 2023 •

edited

Loading

source-paypal-transaction test report (commit `c55c69c747`) - ✅

source-paypal-transaction test report (commit `1f4f418f0a`) - ✅