Skip to content

Fix: don't require --download-path to be readable at parse time#45

Open
zboyles wants to merge 1 commit into
magenta:mainfrom
zboyles:fix-download-path-readable
Open

Fix: don't require --download-path to be readable at parse time#45
zboyles wants to merge 1 commit into
magenta:mainfrom
zboyles:fix-download-path-readable

Conversation

@zboyles

@zboyles zboyles commented Jun 9, 2026

Copy link
Copy Markdown

Fix: don't require --download-path to be readable at parse time

The mrt models init, mrt models download, and mrt checkpoints download commands typed --download-path as click.Path(), which defaults to readable=True. Click probes os.access(path, R_OK) on the default path (~/Documents/Magenta/magenta-rt-v2) at argument-parse time. When that directory exists but is not readable by the process, parsing fails with "Path '...' is not readable." before any command logic runs.

This is hit out of the box on macOS, where ~/Documents is TCC-protected: a terminal that has not yet been granted Documents access fails the probe. A download destination does not need to be readable to be valid (it is created/written on download), so the readability check is inappropriate.

Extract the three duplicated --download-path options into a shared _download_path_option decorator (matching the existing _source_option pattern) and set readable=False on it.

Repro (any OS): point --download-path at an existing dir with mode 000 and run the command; before this change click rejects it as "not readable".

Related Issues

  • None — found while setting up on Apple Silicon. Will open a tracking issue if preferred.

Local Pytests

Note

This change is confined to CLI argument parsing. It does not touch model loading, generation, or numerics, so the model-generation and bit-level parity tests are not applicable here. Verification below targets what the change actually affects.

I ran

# Repro of the bug + fix (any OS): an existing but unreadable destination dir
mkdir -p /tmp/dl_dest_demo && chmod 000 /tmp/dl_dest_demo
python -c "
import click, os
p = '/tmp/dl_dest_demo'
print('os.access R_OK:', os.access(p, os.R_OK))
for rd in (True, False):
    try:
        click.Path(readable=rd).convert(p, None, None)
        print(f'click.Path(readable={rd}) -> accepted')
    except click.BadParameter as e:
        print(f'click.Path(readable={rd}) -> {e}')
"
chmod 755 /tmp/dl_dest_demo && rm -rf /tmp/dl_dest_demo

# All three commands still parse --download-path correctly
mrt models init --help
mrt models download --help
mrt checkpoints download --help

python -m py_compile magenta_rt/cli/models_commands.py

and observed the following output:

os.access R_OK: False
click.Path(readable=True) -> Path '/tmp/dl_dest_demo' is not readable.
click.Path(readable=False) -> accepted

# (each --help prints usage with "--download-path PATH ... [default: .../magenta-rt-v2]"; all exit 0)
# py_compile: no output, exit 0

Benchmark Regression Test

N/A — this change does not affect model loading, generation, or performance-sensitive code paths, so there is no benchmark surface to regress.

The `mrt models init`, `mrt models download`, and `mrt checkpoints download` commands typed `--download-path` as `click.Path()`, which defaults to `readable=True`. Click probes `os.access(path, R_OK)` on the default path (`~/Documents/Magenta/magenta-rt-v2`) at argument-parse time. When that directory exists but is not readable by the process, parsing fails with "Path '...' is not readable." before any command logic runs.

This is hit out of the box on macOS, where `~/Documents` is TCC-protected: a terminal that has not yet been granted Documents access fails the probe. A download destination does not need to be readable to be valid (it is created/written on download), so the readability check is inappropriate.

Extract the three duplicated `--download-path` options into a shared `_download_path_option` decorator (matching the existing `_source_option` pattern) and set `readable=False` on it.

Repro (any OS): point --download-path at an existing dir with mode 000 and run the command; before this change click rejects it as "not readable".
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.

1 participant