From 69f7f8ab22a7c111793745c70b12a3733d5750e7 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Sat, 24 Oct 2026 19:07:41 +0100 Subject: [PATCH] fix(no-ticket): show the real progress of a single-part push upload The upload callback gave the running byte count to ProgressBar.update, which adds its argument. The bar was full after a small part of the file. The callback now moves the bar to the read part of the body, scaled to the file size. The bar does not move back when a rewound body counts again from 0. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++ cloudsmith_cli/cli/commands/push.py | 12 +++++- cloudsmith_cli/cli/tests/test_push.py | 61 +++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0127743c..33d90236 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - After a SAML login, `cloudsmith auth` saves the Workspace to `config.ini`. It writes to the `[profile:NAME]` section of the selected profile, or to `[default]` when you do not select a profile. The CLI writes to the file or directory that `--config-file` gives, or else to your user-level `config.ini`. If the `config.ini` does not exist, the CLI creates it. +### Fixed + +- The `cloudsmith push` progress bar for a single-part upload now shows the real upload progress. Before, it showed 100% long before the upload was complete. + ## [1.30.0] - 2026-10-22 ### Added diff --git a/cloudsmith_cli/cli/commands/push.py b/cloudsmith_cli/cli/commands/push.py index c94141d0..32081412 100644 --- a/cloudsmith_cli/cli/commands/push.py +++ b/cloudsmith_cli/cli/commands/push.py @@ -10,6 +10,7 @@ from datetime import UTC, datetime import click +from requests_toolbelt import MultipartEncoderMonitor from ...core import utils as core_utils from ...core.api.exceptions import ApiException @@ -552,8 +553,15 @@ def upload_file(ctx, opts, owner, repo, filepath, skip_errors, md5_checksum): empty_char=click.style("-", fg="red"), ) as pb: - def progress_callback(monitor): - pb.update(monitor.bytes_read) + def progress_callback(monitor: MultipartEncoderMonitor) -> None: + if monitor.len <= 0: + return + # The body also holds the form fields, so scale it to the + # file. A rewound body counts again from 0, so do not move + # the bar back. + bytes_read = min(max(monitor.bytes_read, 0), monitor.len) + uploaded = bytes_read * filesize // monitor.len + pb.update(max(uploaded - pb.pos, 0)) api_upload_file( upload_url=upload_url, diff --git a/cloudsmith_cli/cli/tests/test_push.py b/cloudsmith_cli/cli/tests/test_push.py index 16deab72..abccb9a1 100644 --- a/cloudsmith_cli/cli/tests/test_push.py +++ b/cloudsmith_cli/cli/tests/test_push.py @@ -1,4 +1,5 @@ # pylint: disable=too-many-lines +import io import json import os import tempfile @@ -8,12 +9,14 @@ import click import pytest +from requests_toolbelt import MultipartEncoder, MultipartEncoderMonitor from ...core.api.exceptions import ApiException from ..commands.push import ( _print_metadata_retry_hint, attach_metadata_to_package, resolve_push_metadata_options, + upload_file, upload_files_and_create_package, validate_metadata_payload, wait_for_package_sync, @@ -1525,3 +1528,61 @@ def test_wait_for_package_sync_json_mode_prints_status_reason_on_failure(capsys) assert envelope["detail"] == status_reason assert envelope["meta"]["stage"] == "Parsing Package Metadata" assert envelope["meta"]["status"] == "Failed" + + +def _push_single_part(tmp_path, failed_reads): + """Push a small file, and return the bar length and the progress per read.""" + filepath = tmp_path / "package.raw" + filepath.write_bytes(b"payload" * 3000) + bars = [] + progress = [] + real_progressbar = click.progressbar + + def quiet_progressbar(**kwargs): + bars.append(real_progressbar(file=io.StringIO(), **kwargs)) + return bars[-1] + + def fake_upload(upload_url, upload_fields, filepath, callback): + for reads_before_failure in (*failed_reads, None): + with open(filepath, "rb") as file_obj: + fields = [*upload_fields.items(), ("file", ("package.raw", file_obj))] + monitor = MultipartEncoderMonitor(MultipartEncoder(fields), callback) + while monitor.read(1024) and len(progress) != reads_before_failure: + progress.append((monitor.bytes_read, monitor.len, bars[0].pos)) + + with ( + patch( + "cloudsmith_cli.cli.commands.push.request_file_upload", + return_value=("id", "https://upload.example.invalid/", {"key": "value"}), + ), + patch( + "cloudsmith_cli.cli.commands.push.api_upload_file", side_effect=fake_upload + ), + patch("click.progressbar", quiet_progressbar), + ): + upload_file( + MagicMock(), + SimpleNamespace(output=None, debug=True), + "owner", + "repo", + str(filepath), + False, + "md5", + ) + + return bars[0].length, progress + + +def test_single_part_progress_is_full_only_when_the_body_is_read(tmp_path): + length, progress = _push_single_part(tmp_path, failed_reads=()) + + for bytes_read, body_length, position in progress: + assert (position == length) == (bytes_read == body_length) + + +def test_single_part_progress_does_not_go_back_on_a_retry(tmp_path): + length, progress = _push_single_part(tmp_path, failed_reads=(10,)) + positions = [position for _, _, position in progress] + + assert positions == sorted(positions) + assert positions[-1] == length