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.
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
Add Acquire collection loader #935
base: main
Are you sure you want to change the base?
Add Acquire collection loader #935
Changes from 1 commit
23347be
08b5027
ee3cb91
c6c9f04
29528b9
a0c5d89
1ba4590
6fd5869
eddc161
5ae9fec
00a606b
4072e20
d3d7f9d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This will be very slow on large tar files, where there will be a double performance penalty. First the entire tar file will be parsed just for this check, then if it exists, it will be parsed again in
__init__
. However, if it doesn't exist, it will still be parsed again in the actual tar loader.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.
Yes I agree, but is there a way to prevent that? As we do need to check if the
fs
directory exists in the file, since otherwise the tar is not an acquire collect and thus need to be handled by theTarLoader
.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.
"Ideally" (not really, since it's only for performance and the code path itself will be more confusing) it's a subpath in the
zip
andtar
loaders. So you piggy back on the detection logic of those, and upon mapping you do a fast check to see if you need to divert logic to Acquire logic (which could exist inloaders/acquire.py
.At least, that's the "best" idea I can come up with right now.