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

feat(file-mode-api): move file uploader to record selector level. #449

Conversation

aldogonzalez8
Copy link
Contributor

Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/12226

Right now, the file uploader is at the schema level, but it needs to be at least at the end of the RecordSelector meaning. If we have client-side filtering, we only want to download the files if we know the record will get synced.

@aldogonzalez8 aldogonzalez8 self-assigned this Mar 27, 2025
@aldogonzalez8 aldogonzalez8 requested a review from maxi297 March 27, 2025 22:21
@github-actions github-actions bot added the enhancement New feature or request label Mar 27, 2025
Copy link
Contributor

@maxi297 maxi297 left a comment

Choose a reason for hiding this comment

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

The code looks good to me so I would say :shipit:

However, should we be concerned about all the source tests failing with Test failed for connector 'source-pokeapi' on step 'BuildConnectorImages'.? I'm not sure how this would be related to this PR but it still feels off

@aldogonzalez8
Copy link
Contributor Author

aldogonzalez8 commented Mar 31, 2025

The code looks good to me so I would say :shipit:

However, should we be concerned about all the source tests failing with Test failed for connector 'source-pokeapi' on step 'BuildConnectorImages'.? I'm not sure how this would be related to this PR but it still feels off

My understanding is that because we are using the test pypi airbyte-protocol-models-dataclasses source, once that change is merged and use prod pypi, it should be able to build the image:

INFO: pip is looking at multiple versions of airbyte-cdk to determine which version is compatible with other requirements. This could take a while.
Stderr:
ERROR: Could not find a version that satisfies the requirement airbyte-protocol-models-dataclasses==0.14.1337.dev1742858109 (from airbyte-cdk) (from versions: 0.13.0, 0.13.1, 0.14.0, 0.14.1, 0.14.2)
ERROR: No matching distribution found for airbyte-protocol-models-dataclasses==0.14.1337.dev1742858109

@aldogonzalez8 aldogonzalez8 merged commit 188f9a5 into aldogonzalez8/poc-emit-file-reference-record Mar 31, 2025
9 of 22 checks passed
@aldogonzalez8 aldogonzalez8 deleted the aldogonzalez8/move-file-uploader-to-record-selector branch March 31, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants