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 Salesforce: Update streams.py for adapting changes with AsyncRetriever #55186

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

btkcodedev
Copy link
Collaborator

What

Closes https://github.com/airbytehq/airbyte-internal-issues/issues/11791

How

In specific:

  • upgrade the CDK version to at least 6.36.3

  • rename the urls_extractor to download_target_extractor, here and here

  • move off the stream_slice interpolation context to use the {{creation_response['id']}}, here

  • make sure the CAT passes, as before

  • make sure the Regression Tests pass, as before

  • no source behaviour changes are expected

@btkcodedev btkcodedev requested a review from a team as a code owner March 4, 2025 13:09
Copy link

vercel bot commented Mar 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 19, 2025 8:21pm

@btkcodedev
Copy link
Collaborator Author

btkcodedev commented Mar 4, 2025

/bump-version type="minor" changelog="Update manifest for adapting changes with AsyncRetriever"

Bump Version job started... Check job output.

✅ Changes applied successfully. (611224c)

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Mar 4, 2025
@maxi297
Copy link
Contributor

maxi297 commented Mar 4, 2025

Tests are failing because of TypeError: AsyncHttpJobRepository.__init__() got an unexpected keyword argument 'download_target_extractor' (and maybe for other reasons). Let me know when this is ready for a review and I'll check it out!

@bazarnov
Copy link
Collaborator

bazarnov commented Mar 4, 2025

Tests are failing because of TypeError: AsyncHttpJobRepository.__init__() got an unexpected keyword argument 'download_target_extractor' (and maybe for other reasons). Let me know when this is ready for a review and I'll check it out!

Because of this, here:

airbyte-cdk = "^5.10.2"

The version should be:

airbyte-cdk = "^6.33.3"

@btkcodedev
Copy link
Collaborator Author

Thanks @bazarnov That helps!!

@maxi297
Copy link
Contributor

maxi297 commented Mar 18, 2025

Hey @btkcodedev, thanks for the contribution! It turns out that this was more complex than anticipated. Hence, I pushed some changes. The tests will still fail. This is expected at this point. We need this fix from the CDK. Once the CDK release is done, we can up the version here and the tests should be green. At that point, I'll ask a review from another extensibility member

@maxi297
Copy link
Contributor

maxi297 commented Mar 19, 2025

This recent change has created a regression where we don't retry on timed out jobs. I've started a thread internally to try to understand why we have this change

@maxi297
Copy link
Contributor

maxi297 commented Mar 19, 2025

Everything has been updated. CATs are failing but they are failing the same way on master so I think I'm fine with this. @bazarnov can you have a review on this?

@maxi297 maxi297 requested a review from bazarnov March 19, 2025 18:36
Copy link
Collaborator

@bazarnov bazarnov left a comment

Choose a reason for hiding this comment

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

Looks great!

The CAT issues are about the "expected records." Some "IDs" are not matching, which is a relatively straightforward problem that can be easily resolved by updating the exptected_records. @btkcodedev

@bazarnov
Copy link
Collaborator

bazarnov commented Mar 19, 2025

/approve-regression-tests

Check job output.

✅ Approving regression tests

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/salesforce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants