Skip to content

CLI: curate --sql-file and up --data-root answer bad path input with a traceback and exit 1 #145

Description

@chiruu12

What happens

Two commands still answer bad path input with a traceback and exit 1. Measured on ccaa8ad,
same harness as #124:

curate --sql-file <a directory>    exit=1  IsADirectoryError: [Errno 21] Is a directory: '/tmp/probe/adir'
up --data-root <a file>            exit=1  NotADirectoryError: [Errno 20] Not a directory: '/tmp/probe/afile/runtime/dags'

Every other path flag on those commands prints one line and exits 2.

curate --sql-file

_command_curate (cli.py:788) reads the file inside a try that catches
(ValueError, FileNotFoundError):

sql = arguments.sql if arguments.sql is not None else arguments.sql_file.read_text()

read_text() on a directory raises IsADirectoryError, which subclasses OSError and not
FileNotFoundError, so it walks straight past the handler at cli.py:799.

Note this is the same class boundary #102 called out, seen from the other side. There the
fix was to keep raising FileNotFoundError so existing handlers keep working. Here the
exception comes from pathlib rather than from our code, so the handler is what has to
move.

up --data-root

A --data-root that is a regular file reaches render_bundle and fails inside mkdir
(_bundle.py:876):

for directory in (dags_dir, logs_dir, user_dir):
    directory.mkdir(parents=True, exist_ok=True)

The default bundle dir is <data-root>/runtime, so a file data root makes every one of
those three mkdir calls raise NotADirectoryError. _command_up catches
FileNotFoundError and (ComposeError, TimeoutError), neither of which covers it.

Nothing has been rendered or started when this happens, so it is bad input, exit 2, by the
same reasoning as #84, #90 and #95.

Definition of done

  1. curate --sql-file pointed at a directory prints one line prefixed curate: and exits
    2, with no traceback.
  2. up --data-root pointed at a regular file prints one line prefixed up: and exits 2,
    with no traceback, before anything is rendered.
  3. Tests cover both, in tests/test_runtime_cli.py beside the existing exit-code cases.

Two things to get right

Pattern to copy

_command_up already wraps RuntimeConfig(...) in its own try/except ValueError -> return 2
(#84), and #95 added a separate except FileNotFoundError block rather than widening an
existing tuple.

Validation

uv sync --locked --all-extras
uv run ruff check --fix
uv run ruff format
uv run ty check
uv run pytest -q

Notes

Found by sweeping every command and path flag with a directory where a file is expected and
a file where a directory is expected. Those two were the only survivors; everything else
already answers with one line and exit 2.

I am happy to do this one and will wait to be assigned first.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions