feat(install): improve logging, error handling, and bulk-loop resilience#126
Merged
Conversation
- Add --verbose flag that echoes shell commands and shows curl progress - Bulk install no longer aborts on a single package failure; failed tools are collected and reported in a summary, exiting non-zero if any failed - Wrap download errors with package name and URL context (IntegrityError passes through unchanged since it already carries context) - Replace shell 'rm -rvf' on --force with shutil.rmtree - Surface a warning when a binary is installed without an sha256 digest - Move metadata_cache.clear() into a finally block - Add rich.console status output (OK/SKIP/fail) matching update/digests - Add [i/N] progress header and per-package status for bulk installs - Add tests for error wrapping, verbose passthrough, force removal, already-installed skip, and bulk-loop resilience
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
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.
Summary
Improves the
tools.installtask with better verbose logging, resilient bulk installs, and clearer error context.Motivation
A review of
tools.installfound that: (1) one package failure aborted the entire bulk install, (2) download errors lost package context, (3) there was no--verbosemode or per-step status, and (4) the output style was inconsistent with theupdate/digeststasks. This addresses all four.Changes
Logging & verbosity
--verboseflag ontools.install: echoes underlying shell commands (curl, tar, chmod, etc.) and shows curl's download progress meterprint()tologging.getLogger(infofor download/verify steps)rich.console(OK/SKIP/✗) for consistency withupdate.pyanddigests.py[i/N] installing <name>progress header for bulk installs and a finalSummary: X installed, Y failedError handling
Exit(1)) if any failedFailed to install '<name>' from <url>: <cause>.IntegrityErrorpasses through unwrapped (already carries context)check_required_toolserror now names the package that needed the missing tool--forceremoval switched fromc.run('rm -rvf ...')toshutil.rmtree(removes a shell-interpolation surface)metadata_cache.clear()moved into afinallyblock so it runs on success or failureIntegrity visibility
WARNING(always visible) when a binary is installed without an sha256 digest configured, rather than skipping verification silentlyFiles changed
tasks/lib/downloader.py— verbose/echo support, logging, verify-step visibilitytasks/tools/_install.py— package-context error wrapper, verbose passthrough,shutil.rmtree, rich console outputtasks/tools/install.py—--verboseflag, resilient bulk loop, summary,finallycache cleartests/test_install_task.py— 10 new tests (error wrapping, IntegrityError pass-through, verbose passthrough, force removal, already-installed skip, bulk-loop resilience, cache-clear-in-finally)tests/test_sha256.py— updatedfake_runsignatures to accept the newecho=kwargVerification
ruff check+ruff format --check— cleanpytest— 164 passed (+13 new tests)_install.py95%,install.py94%Checklist
check_required_toolsgained an optional param;install_single_packagegained optionalverbose)