Fix pass through adapter when using minio (s3) #132
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
masaball
approved these changes
Jan 31, 2024
There was a problem hiding this comment.
These changes look good to me. I do agree that there should be tests for this though.
Looking at the test suite, I don't think the ffmpeg adapter has tests for the s3 case either. Maybe we make a new issue to circle back and add s3 test cases for both adapters? But it can be harder adding proper test cases in isolation of tweaking the main code...
If you feel like you have the cycles it would be good to add tests for this but I'm okay with merging this as is and circling back for that when we are not trying to prep an Avalon release.
|
I created #133 to cycle back to creating these tests later. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A couple of changes were needed to make the pass through adapter usable with minio inside avalon:
download_filemethod supplied byaws-sdk-s3for transferring derivative to working directory