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
remote: use progress bar when paginating #3532
remote: use progress bar when paginating #3532
Changes from all commits
48a8078
1c73fe7
d9ca41b
d84fd8c
56c2942
b200d7d
ba1dc78
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.
unused?
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.
progress_callback
is unused in ssh and local remotes, but the parameter was added to both so that overriddenlist_cache_paths()
in ssh/local still match the abstract method defined inRemoteBASE.list_cache_paths()
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 but presumably it should be used now that it's added?
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.
both local and ssh have their own overridden
cache_exists()
implementation and neither one useslist_cache_paths()
at all duringcache_exists()
. The only timelist_cache_paths()
would get called for either local or ssh remotes would be forRemoteBASE.all()
during advc gc
call.all()
does not currently display any progressbars at all, but now that I'm thinking about this, it probably shouldThere 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.
indeed; though I'm not sure in principle about having a function exists which takes a non-null
progress_callback
and ignores it. This implies a bar will be displaying hanging at 0% on screen and then eventually suddenly disappear.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.
list_cache_paths()
for local and ssh have been updated to useprogress_bar
(if it is set) to keep them consistent with the other remotes. A separate issue (#3543) has been filed for updating thedvc gc
/RemoteBASE.all()
behavior and will be addressed in a follow-up PR.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.
btw just to clarify - every time I've said "unused?" it's more a reminder to address at some point (could be in a future PR) rather than an expectation to address in this PR.