Skip to content

feat(cli): add flash update command with passive update check#237

Open
deanq wants to merge 5 commits intomainfrom
deanq/ae-2299-cli-flash-update
Open

feat(cli): add flash update command with passive update check#237
deanq wants to merge 5 commits intomainfrom
deanq/ae-2299-cli-flash-update

Conversation

@deanq
Copy link
Member

@deanq deanq commented Mar 2, 2026

Summary

  • Add flash update command for self-updating runpod-flash to latest or a specific version via PyPI
  • Prefer uv pip install when uv is on PATH, fall back to python -m pip install otherwise
  • Add passive background update check that runs on CLI invocation (at most once per 24h, cached to disk) and prints a one-line stderr notice if a newer version exists
  • Background check excluded from flash run (long-running), flash update (already managing versions), and CI/headless environments

Test plan

  • Run flash update -- verify it checks PyPI and reports current version
  • Run flash update --version 1.4.0 -- verify it installs specific version (uses uv)
  • Run flash build --help -- verify passive check triggers (inspect ~/.config/runpod/update_check.json)
  • Run flash run --help -- verify passive check does NOT trigger
  • Set FLASH_NO_UPDATE_CHECK=1 and run any command -- verify check is skipped
  • All 1307 tests pass, 74.18% coverage

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a self-update flow to the Flash CLI along with a passive, cached update notification that runs in the background on most CLI invocations.

Changes:

  • Introduces flash update command to install the latest (or a specified) runpod-flash version from PyPI.
  • Adds a passive background update checker with a 24h on-disk cache and an atexit notice on stderr when a newer version is available.
  • Wires the background checker into the CLI entrypoint while excluding flash run and flash update.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/runpod_flash/cli/commands/update.py New flash update command plus helper functions for PyPI lookup and pip install.
src/runpod_flash/cli/update_checker.py New background update-checker module with disk cache + atexit notice.
src/runpod_flash/cli/main.py Registers the new update command and triggers the passive update check for most subcommands.
tests/unit/cli/commands/test_update.py Unit/integration-style tests for flash update behavior.
tests/unit/cli/test_update_checker.py Unit tests for caching, background check behavior, and notice printing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@deanq
Copy link
Member Author

deanq commented Mar 2, 2026

Re: no-subcommand / --help / --version skipping the check (comment on main.py:97)

Intentional. The no-subcommand path only renders the welcome panel and exits — there's no meaningful work for the check to overlap with, and the notice would print immediately after the panel (odd UX). Same for --version which raises typer.Exit() before the check. The gating stays as-is; I'll clarify the PR description.

Addressing the other 4 items in code:

  • _fetch_pypi_metadata: catch JSONDecodeError/KeyError/UnicodeDecodeError and re-raise as RuntimeError
  • _run_check: treat missing/empty latest_version in fresh cache as stale (fall through to fetch)
  • start_background_check: skip when neither stdout nor stderr is a TTY
  • start_background_check: add idempotency guard to prevent duplicate threads/atexit handlers

Copy link
Member Author

@deanq deanq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All 5 review items addressed in 3e4af42:

  1. main.py:97 (no-subcommand skip) — Intentional by design. No-args shows the welcome panel and exits immediately; --version raises typer.Exit() before the check. Neither has meaningful work to overlap with.

  2. update.py:58 (JSON/key errors) — Fixed. _fetch_pypi_metadata now catches JSONDecodeError, UnicodeDecodeError, and KeyError, re-raising as RuntimeError with user-friendly messages.

  3. update_checker.py:100 (missing latest_version in fresh cache) — Fixed. Missing or empty latest_version in a fresh cache is now treated as stale, falling through to a PyPI fetch.

  4. update_checker.py:143 (non-TTY skip) — Fixed. Added _is_interactive() helper; skips when neither stdout nor stderr is a TTY.

  5. update_checker.py:147 (idempotency) — Fixed. Added _started flag with _start_lock so start_background_check() only runs once per process.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@runpod-Henrik runpod-Henrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — PR #237: flash update + background update checker

Clean implementation. Three issues found:


Bug 1 (HIGH): uv pip install fails without active venv

update.py_build_install_command()

if shutil.which("uv"):
    return ["uv", "pip", "install", package_spec, "--quiet"]

uv pip install requires an active venv (VIRTUAL_ENV set or .venv discoverable). If flash is installed system-wide and uv is on PATH, this fails with exit code 2. The pip fallback works correctly because it uses sys.executable.

Fix: Pass --python sys.executable:

return ["uv", "pip", "install", "--python", sys.executable, package_spec, "--quiet"]

Bug 2 (MEDIUM): _started = True set before env-var guards — permanently disables update checks

update_checker.pystart_background_check()

with _start_lock:
    if _started:
        return
    _started = True      # <-- set HERE

if os.getenv("FLASH_NO_UPDATE_CHECK"):
    return               # _started=True, no thread started, no atexit

If called with CI=1 or FLASH_NO_UPDATE_CHECK=1, _started becomes True but no thread/atexit is registered. If the env var is later cleared (test suites), subsequent calls are silently no-oped. Move _started = True to after the guards, just before thread.start().


Bug 3 (LOW): _parse_version crashes on PEP 440 pre-release versions

update.py_parse_version()

return tuple(int(part) for part in version.split("."))

PyPI info.version can be 1.5.0rc1 or 1.5.0a1. int("0rc1") raises ValueError. In update_command the except ValueError: pass handles it, but in update_checker._run_check() the bare except Exception: pass silently skips the entire comparison — no update notice is printed even if a newer stable version exists.

Fix: Use packaging.version.Version (already a transitive dep):

from packaging.version import Version
def _parse_version(version: str) -> Version:
    return Version(version)

deanq added a commit that referenced this pull request Mar 4, 2026
- Add _compare_versions() with zero-padding for differing tuple lengths
- Fix _started flag ordering in start_background_check (set after skip checks)
- Replace Linux-specific /proc test path with monkeypatch
- Fix missing newline in _print_update_notice output
- Add TestCompareVersions with 8 test cases covering edge cases
Copy link
Contributor

@runpod-Henrik runpod-Henrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review by Henrik's AI-Powered Bug Finder

Checked ed1c2e3 against our three original items:

Bug 2 (_started flag): FIXED

_started = True now set after all skip checks, inside the lock. Thread start + atexit.register also moved inside the lock — cleaner than before. Good.

Bug 3 (_parse_version): MITIGATED

_compare_versions() with zero-padding handles the tuple-length edge case ((2, 0) == (2, 0, 0)). 8 solid test cases. However, _parse_version("1.5.0rc1") still raises ValueError — the except Exception: pass in _run_check() swallows it, so users on a pre-release version get no update notice for the next stable release. Low priority, but worth a note.

Bug 1 (uv pip install without venv): STILL OPEN — follow-up item

_build_install_command() still runs bare uv pip install which requires an active venv. If flash is installed system-wide and uv is on PATH, this fails with exit code 2. Not blocking since the error message is clear and flash update is non-critical. Fix for follow-up: ["uv", "pip", "install", "--python", sys.executable, ...]

Bonus fixes

  • /proc test path → monkeypatch (cross-platform)
  • Missing newline in update notice output
  • Both well done.

Approving — Bug 2 fix was the important one and it's solid. Bug 1 is a minor edge case suitable for a follow-up PR.

🤖 Reviewed by Henrik's AI-Powered Bug Finder

deanq added 5 commits March 4, 2026 23:12
Adds `flash update` to upgrade to latest PyPI version and
`flash update --version X.Y.Z` to pin a specific version.
Uses stdlib only (urllib.request + subprocess) with no new deps.
Spawn a daemon thread on CLI invocation that checks PyPI (at most once
per 24h, cached to ~/.config/runpod/update_check.json). An atexit
handler prints a one-line notice to stderr if a newer version exists.
The thread never blocks the command -- slow networks are silently skipped.

Excluded from check: flash run (long-running), flash update (already
managing versions), no-subcommand (help panel). Opt-out via
FLASH_NO_UPDATE_CHECK or CI env vars.
_run_pip_install replaced with _run_install that prefers `uv pip install`
when uv is on PATH, falling back to `python -m pip install` otherwise.
Fixes "No module named pip" error in uv-managed environments.
- _fetch_pypi_metadata: catch JSONDecodeError, UnicodeDecodeError, and
  KeyError from malformed PyPI responses, re-raise as RuntimeError
- _run_check: treat missing/empty latest_version in fresh cache as stale
  so a corrupted cache does not suppress checks for 24h
- start_background_check: skip when neither stdout nor stderr is a TTY
  to avoid threads and notices in piped output
- start_background_check: add idempotency guard to prevent duplicate
  threads and atexit handlers on repeated calls
- Add _compare_versions() with zero-padding for differing tuple lengths
- Fix _started flag ordering in start_background_check (set after skip checks)
- Replace Linux-specific /proc test path with monkeypatch
- Fix missing newline in _print_update_notice output
- Add TestCompareVersions with 8 test cases covering edge cases
@deanq deanq force-pushed the deanq/ae-2299-cli-flash-update branch from ed1c2e3 to a4ce7d1 Compare March 5, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants