fastapi / typer

Typer, build great CLIs. Easy to code. Based on Python type hints.
https://typer.tiangolo.com/
MIT License
15.44k stars 656 forks source link

[BUG] Non-existent path as default value for an option of type Path raises error #226

Open OnkelTem opened 3 years ago

OnkelTem commented 3 years ago

Python: 3.8 Typer: 0.3.2

I have an optional path option of type Path and I want to assign it a default value:

DEFAULT_MEDIA_DIR = Path(".") / "non-existent-dir"

def cli(
    path: Path = typer.Option(
        DEFAULT_MEDIA_DIR,
        help="Path to media download location",
        exists=True,
        file_okay=False,
        dir_okay=True,
        writable=True,
        readable=True,
        resolve_path=False,
    ),
):
    pass

But it fails with error:

Traceback (most recent call last):
  File "/projects/project/.venv/lib/python3.8/site-packages/click/types.py", line 608, in convert
    st = os.stat(rv)
FileNotFoundError: [Errno 2] No such file or directory: 'non-existent-dir'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/projects/project/.venv/bin/telegram_veytoybot", line 5, in <module>
    app()
  File "/projects/project/.venv/lib/python3.8/site-packages/typer/main.py", line 214, in __call__
    return get_command(self)(*args, **kwargs)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 781, in main
    with self.make_context(prog_name, args, **extra) as ctx:
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 700, in make_context
    self.parse_args(ctx, args)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1048, in parse_args
    value, args = param.handle_parse_result(ctx, opts, args)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1623, in handle_parse_result
    value = self.full_process_value(ctx, value)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1965, in full_process_value
    return Parameter.full_process_value(self, ctx, value)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1592, in full_process_value
    value = self.get_default(ctx)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1917, in get_default
    return Parameter.get_default(self, ctx)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1534, in get_default
    return self.type_cast_value(ctx, rv)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1568, in type_cast_value
    return _convert(value, (self.nargs != 1) + bool(self.multiple))
  File "/projects/project/.venv/lib/python3.8/site-packages/click/core.py", line 1565, in _convert
    return self.type(value, self, ctx)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/types.py", line 46, in __call__
    return self.convert(value, param, ctx)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/types.py", line 614, in convert
    self.path_type, filename_to_ui(value)
  File "/projects/project/.venv/lib/python3.8/site-packages/click/_compat.py", line 474, in filename_to_ui
    value = value.encode("utf-8", "surrogateescape").decode("utf-8", "replace")
AttributeError: 'PosixPath' object has no attribute 'encode'

If I remove the default value and set it to None, then it goes well.

I found only one workaround: set exists=False and check for existence in the body of the function.

daddycocoaman commented 3 years ago

This is the expected behavior. When you pass exists=True, you are telling Typer that the value that is passed needs to exist.

When you pass exists=False, you are telling Typer that the value does not need to be checked for existence.

OnkelTem commented 3 years ago

When you pass exists=True, you are telling Typer that the value that is passed needs to exist.

I think that should just check for existence and not raise unhandled fatal error.

I guess the problem lies underneath, in Click.

graue70 commented 3 years ago

When you pass exists=True, you are telling Typer that the value that is passed needs to exist.

I think that should just check for existence and not raise unhandled fatal error.

What do you expect to happen instead? What does "just check for existence" actually mean?

OnkelTem commented 3 years ago

Sorry don't you really get what's going on folks? :) The application just crashes. No nice message in the cli. It crashes.

What does "just check for existence" actually mean?

Return usage, Problem. Not the crash.

OnkelTem commented 3 years ago

I remind, if I pass a wrong path via cli params - it works just well, it shows a regular error message about improper usage of params of the tool. But if I try to default it to unexisting dir, it crashes. And my defaults are not hardcoded, they're read from a config file.

graue70 commented 3 years ago

You can circumvent the problem if you use a str instead of a pathlib.Path, e.g. like this:

def cli(
    path: Path = typer.Option(
        str(DEFAULT_MEDIA_DIR),
        exists=True,
    ),
):
    pass

You are right in saying that the problem lies in click. See this example:

import click
from pathlib import Path

DEFAULT_DIR = Path(".") / "non-existing-dir"  # raises AttributeError
# DEFAULT_DIR = "./non-existing-dir"  # works fine

@click.command()
@click.argument("path", default=DEFAULT_DIR, type=click.Path(exists=True))
def main(path):
    click.echo(click.format_filename(path))

if __name__ == "__main__":
    main()

This is probably related to https://github.com/pallets/click/issues/405.

alexreg commented 2 years ago

There is no longer any crash, so this should probably be closed.

(I'm not sure if https://github.com/pallets/click/issues/405 is actually related to it, but in any case that is now implemented too in Click 8.x.)

OnkelTem commented 2 years ago

Currently I'm not at my workstation so cannot check it.