Fix progress bar resetting during import/export operations#14057
Fix progress bar resetting during import/export operations#14057rtibbles wants to merge 1 commit intorelease-v0.19.xfrom
Conversation
Build Artifacts
|
This commit fixes issue #5501 where the progress bar would go from 0% to 100%, reset to 0%, then go to 100% again during import/export operations. The root cause was that multi-phase operations (channel database + content files) each independently reset and tracked their own progress. Changes: - Create DiskChannelResourceExportManager in resource_export.py that coordinates progress tracking across channel database and content file export phases - Add import_channel_database parameter to ResourceImportManagerBase and its subclasses to optionally include channel database import with coordinated progress tracking - Update exportchannel and exportcontent management commands to use the new export manager - Update diskexport task to use the export manager - Update remoteimport and diskimport tasks to use the new import_channel_database parameter - Add tests for progress tracking coordination The fix ensures that: 1. Total bytes are calculated upfront for ALL phases 2. Progress is initialized once at the start 3. Progress updates incrementally through all phases without resetting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
e970047 to
e5e0a82
Compare
rtibblesbot
left a comment
There was a problem hiding this comment.
Thanks for tackling this long-standing UX issue! The approach of coordinating progress tracking across import/export phases is solid, and the new DiskChannelResourceExportManager is a clean design. CI is fully green and the new tests cover the key scenarios well.
I have a few observations below — one suggestion about a behavioral difference from the original code, and a couple of minor items.
Summary of findings:
- 1 suggestion (upgrade DB path handling)
- 2 nitpicks (progress edge case, closure variable capture)
Let me know if you have questions on any of these!
| # Clear any previously set channel availability stats for this channel. | ||
| clear_channel_stats(self.channel_id) | ||
|
|
||
| def do_channel_database_import(self): |
There was a problem hiding this comment.
suggestion: The previous code in transfer_channel() handled an upgrade path: it checked for a previously-downloaded channel DB at get_upgrade_content_database_file_path() and, if present, used COPY_METHOD from that file instead of re-downloading. It also cleaned up that upgrade file after import.
This new do_channel_database_import() always downloads/copies directly from the source without checking for an existing upgrade DB. This means if diff_stats had previously downloaded a new channel DB to the upgrade path (e.g. during a version comparison workflow), the new code will:
- Re-download/copy the DB instead of using the cached version
- Leave the upgrade file on disk (never cleaned up)
This may be acceptable since the remoteimport/diskimport tasks are the "combined" workflow and the standalone remotechannelimport/diskchannelimport tasks still use transfer_channel(). But it's worth confirming this is intentional — especially on resource-constrained devices where re-downloading could be costly.
| # We intentionally overestimate (10000x) because reducing the total later | ||
| # just makes progress "speed up", whereas underestimating would cause | ||
| # progress to appear to go backwards. | ||
| CONTENT_SIZE_ESTIMATE_MULTIPLIER = 10000 |
There was a problem hiding this comment.
nitpick: The 10000x multiplier comment is well-reasoned — overestimating so progress "speeds up" rather than going backwards is a good UX trade-off. However, this multiplier will cause the initial progress during the channel DB download phase to appear nearly frozen (the DB phase represents only 1/10001 of the estimated total). For a large channel DB download that takes significant time, the progress bar may sit at ~0% for a while before jumping when the actual content size is resolved.
Not necessarily something to change, but worth noting in case testers observe this behavior.
|
|
||
| # Total bytes is sum of both phases (if not manifest_only) | ||
| if self.manifest_only: | ||
| self.total_bytes = self.channel_db_size |
There was a problem hiding this comment.
nitpick: When manifest_only=True and export_channel_database=False (as in exportcontent --manifest-only), total_bytes will be 0 since channel_db_size=0. This means start_progress(total=0) is called. The original code didn't explicitly call start_progress in this case.
This is unlikely to cause issues since the manifest-only path completes quickly, but it's a minor behavioral difference from the original code.
Summary
Fixes issue #5501 where the progress bar would go from 0% to 100%, reset to 0%, then go to 100% again during import/export operations.
Root cause: Multi-phase operations (channel database + content files) each independently reset and tracked their own progress.
Solution:
DiskChannelResourceExportManagerthat coordinates progress tracking across both export phasesimport_channel_databaseparameter to import managers to include channel database import with coordinated progress trackingThe fix ensures:
References
Fixes #5501
Reviewer guidance
resource_export.py(new file) andresource_import.pytasks.pynow uses the new managers instead of separateexport_channel/export_contentcallstest_import_export.py🤖 Generated with Claude Code