feat(download): add overall progress bar for multi-file downloads#76
Open
Prakhar54-byte wants to merge 1 commit into
Open
feat(download): add overall progress bar for multi-file downloads#76Prakhar54-byte wants to merge 1 commit into
Prakhar54-byte wants to merge 1 commit into
Conversation
Wraps the URL iteration list in _download_files with a tqdm progress bar to display overall completion progress, which complements the existing per-file byte-level progress bars. Closes #<FEATURE-PROGRESS-BAR>
📝 WalkthroughWalkthroughThe ChangesOverall Progress Bar
Estimated code review effort: 1 (Trivial) | ~3 minutes Related issues: Suggested labels: enhancement Suggested reviewers: Poem: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
databusclient/api/download.py (1)
544-547: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInner per-file bar not updated to
position=1; may conflict with outerposition=0bar.Issue
#71's suggested design pins the outer bar toposition=0and the per-file bar toposition=1. This PR only setsposition=0here; the per-filetqdmcall in_download_file(line 470) still omitspositionand keeps the defaultleave=True. With an explicitly pinned outer bar, tqdm's automatic position allocation for the inner bar may not coordinate correctly, and sinceleave=Trueleaves each closed file-bar printed, the overall bar could end up interleaved/overwritten as multiple leftover lines accumulate during a large batch download.♻️ Suggested fix
progress_bar = tqdm(total=total_size_in_bytes, unit="iB", unit_scale=True) + progress_bar = tqdm(total=total_size_in_bytes, unit="iB", unit_scale=True, position=1, leave=False)Please verify this doesn't cause visual glitches (overlapping/duplicated lines) when running against a real multi-file download.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@databusclient/api/download.py` around lines 544 - 547, The progress bar layout is only partially pinned, which can cause the outer and per-file `tqdm` bars to overlap or leave duplicated lines during multi-file downloads. Update `_download_file`’s inner `tqdm` call to explicitly use `position=1` and set it to not persist after completion, while keeping the outer loop in `download` at `position=0`. Make sure the `tqdm` usage in both `download` and `_download_file` follows the intended two-line layout consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@databusclient/api/download.py`:
- Around line 544-547: The progress bar layout is only partially pinned, which
can cause the outer and per-file `tqdm` bars to overlap or leave duplicated
lines during multi-file downloads. Update `_download_file`’s inner `tqdm` call
to explicitly use `position=1` and set it to not persist after completion, while
keeping the outer loop in `download` at `position=0`. Make sure the `tqdm` usage
in both `download` and `_download_file` follows the intended two-line layout
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1761aa0c-0f2d-47c3-a2fe-fc46cf57d08f
📒 Files selected for processing (1)
databusclient/api/download.py
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Resolves #71 the requested feature for an overall progress bar.
This PR wraps the URL iteration list in
_download_files()with atqdmprogress bar (withposition=0) to display the overall completion progress across all files, which complements the existing per-file byte-level progress bars.Summary by CodeRabbit