- Notifications
You must be signed in to change notification settings - Fork 4.9k
✨feat(source-sftp-bulk): adjust file record message protocol #57514
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(source-sftp-bulk): adjust file record message protocol #57514
Conversation
| The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| | ||
| @pytest.mark.slow | ||
| @pytest.mark.limit_memory("10 MB") | ||
| @pytest.mark.limit_memory("11 MB") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we changed the size here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After play for a few minutes with this locally, I also got a consistent message (not increasing) in the master branch as in the dev branch:
Test was limited to 10.0MiB but allocated 10.6MiB
So, it's more a matter of local memory resources than an issue with the changes. Then, it's safe to increase this to avoid annoying the local developer.
…-record-message-protocol
…-record-message-protocol
What
This PR updates the
source-sftp-bulkconnector to comply with the latest file transfer record protocol.Resolves: https://github.com/airbytehq/airbyte-internal-issues/issues/12342
How
source-sftp-bulk: connector's implementation to align with the latest protocol specifications.Review guide
1.
airbyte-integrations/connectors/source-sftp-bulk/source_sftp_bulk/stream_reader.py: returns file reference and file metadata.User Impact
Can this PR be safely reverted and rolled back?