[DOP-22144] Change logic of applying limits #327
Merged
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.
Change Summary
After thinking on #326 (review), I've desided that
FileConnection.walk
andFileConnection.listdir
implementation should be changed.Previously: iterate over all files in the directory, include file to resulting list, stop if limit is reached.
Now: iterate over all files in the directory, check if limit is reached, include file only if it is not.
This is because if user have files list:
and set limit
TotalFileSize("15KB")
, FileDownloader/FileMover should download only file1.txt and skip both file2.txt and file3.txt (because 10KB+20KB already exceeds the limit of 15KB). Previous implementation lead to downloading file2.txt (as limit internal stage recorded 10KB which is less than the limit), and skipping only file3.txt.This is a possibly breaking change for developers of custom file limits, as now
.is_reached
should returnTrue
after limit is reached, and not before.Related issue number
Checklist
docs/changelog/next_release/<pull request or issue id>.<change type>.rst
file added describing change(see CONTRIBUTING.rst for details.)