litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.65k stars 384 forks source link

Enhancement: Add `--pythonpath` support to CLI #1449

Closed pawelad closed 1 year ago

pawelad commented 1 year ago

Summary

Hi!

I'm a big fan of the src/ based layouts, which unfortunately doesn't seem to be well supported in the starlite CLI interface. Adding --pythonpath (similarly to gunicorn, pytest and others) would help.

If there's support for the idea, I can try and help with a PR.

Basic Example

Given this layout:

- src/
  - foobar/
    - __init__.py
    - app.py

This (predictably) doesn't work:

$ starlite --app src.foobar.app:application info

This works, but I don't think it's nice DX:

$ PYTHONPATH=src starlite --app foobar.app:application info

What I'd like is this:

$ starlite --pythonpath src --app foobar.app:application info
$ # Actually, I'd like to skip the `:application` part as well, but that's for another issue : )

Drawbacks and Impact

I am not aware of any drawbacks.

Unresolved questions

I am not aware of any unresolved questions.

provinzkraut commented 1 year ago

Hey @pawelad, thanks for bringing this up!

Adding --pythonpath would certainly be a good option, but I think we can also just support the src pattern by default.

Actually, I'd like to skip the :application part as well, but that's for another issue : )

You can! If you app is in any of the default locations (app, application), you won't need to specify --app at all. There are quite a few patterns supported for discovering the application instance: app, application, create_app callables, any callable that returns Starlite.

I'm afraid making this any more generic is going to have some undesirable side effects.

patrickarmengol commented 1 year ago

This (predictably) doesn't work:

$ starlite --app src.foobar.app:application info

Did something change since this issue was created? That command seems to work for me. Running an identical project structure with a venv activated; on Litestar 2.0.0a4

Alternatively, you could install the module locally with pip install -e . and instead run litestar --app foobar.app:application info

pawelad commented 1 year ago

Adding --pythonpath would certainly be a good option, but I think we can also just support the src pattern by default.

That would work for me, but I guess could be considered "too magical" for some (I'm guessing that's why other tools use the more explicit --pythonpath option).

From my perspective, any option without the ugly PYTHONPATH=src is a win : )

Actually, I'd like to skip the :application part as well, but that's for another issue : )

You can! If you app is in any of the default locations (app, application), you won't need to specify --app at all. There are quite a few patterns supported for discovering the application instance: app, application, create_app callables, any callable that returns Starlite.

Sorry, I should've been more clear - if you specify the --app parameter, you also need to specify the name of the application instance as well, even if it's named app / application otherwise you get an error:

$ PYTHONPATH=src starlite --app calculon.app:application info
┌──────────────────┬──────────────────────┐
│ Starlite version │ 1.51.9               │
│ Debug mode       │ Enabled              │
│ CORS             │ Disabled             │
│ CSRF             │ Disabled             │
│ Request caching  │ Enabled              │
│ OpenAPI          │ Enabled path=/schema │
│ Compression      │ Disabled             │
│ Plugins          │ PiccoloORMPlugin     │
└──────────────────┴──────────────────────┘

$ PYTHONPATH=src starlite --app calculon.app info            
Traceback (most recent call last):
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/bin/starlite", line 5, in <module>
    from starlite import __main__
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/starlite/__main__.py", line 3, in <module>
    starlite_group()  # pylint: disable=E1120  # pragma: no cover
    ^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1654, in invoke
    super().invoke(ctx)
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/starlite/cli/main.py", line 15, in starlite_group
    ctx.obj = StarliteEnv.from_env(app_path)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/starlite/cli/utils.py", line 97, in from_env
    loaded_app = _load_app_from_path(app_path)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/starlite/cli/utils.py", line 216, in _load_app_from_path
    module_path, app_name = app_path.split(":")
    ^^^^^^^^^^^^^^^^^^^^^
ValueError: not enough values to unpack (expected 2, got 1)

Ideally, I think it should also support providing the app path but then using the default values you mention to discover the app instance.

provinzkraut commented 1 year ago

Ideally, I think it should also support providing the app path but then using the default values you mention to discover the app instance.

That's a reasonable addition. Want to open a separate issue for this?

I'm going to close this one, since it was implemented in #1506.

pawelad commented 1 year ago

This (predictably) doesn't work:

$ starlite --app src.foobar.app:application info

Did something change since this issue was created? That command seems to work for me. Running an identical project structure with a venv activated; on Litestar 2.0.0a4

I upgraded to litestar-2.0.0a5 to double check, but as far as I can see it's behaving the same way:

$ litestar --app src.calculon.app:application info 
Using Litestar app from env: 'src.calculon.app:application'
Traceback (most recent call last):
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/bin/litestar", line 5, in <module>
    from litestar import __main__
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/__main__.py", line 3, in <module>
    litestar_group()  # pragma: no cover
    ^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/_utils.py", line 196, in wrapped
    ctx.obj = ctx.obj()
              ^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/main.py", line 17, in <lambda>
    ctx.obj = lambda: LitestarEnv.from_env(app_path)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/_utils.py", line 94, in from_env
    loaded_app = _load_app_from_path(app_path)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/_utils.py", line 225, in _load_app_from_path
    module = importlib.import_module(module_path)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/Dev/Python/calculon/src/calculon/app.py", line 5, in <module>
    from calculon import __version__, __description__
ModuleNotFoundError: No module named 'calculon'

$ litestar --app calculon.app:application info 
Using Litestar app from env: 'calculon.app:application'
Traceback (most recent call last):
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/bin/litestar", line 5, in <module>
    from litestar import __main__
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/__main__.py", line 3, in <module>
    litestar_group()  # pragma: no cover
    ^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/_utils.py", line 196, in wrapped
    ctx.obj = ctx.obj()
              ^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/main.py", line 17, in <lambda>
    ctx.obj = lambda: LitestarEnv.from_env(app_path)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/_utils.py", line 94, in from_env
    loaded_app = _load_app_from_path(app_path)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/envs/calculon/lib/python3.11/site-packages/litestar/cli/_utils.py", line 225, in _load_app_from_path
    module = importlib.import_module(module_path)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pawelad/.pyenv/versions/3.11.2/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'calculon'

$ PYTHONPATH=src litestar --app calculon.app:application info
Using Litestar app from env: 'calculon.app:application'
┌──────────────────┬──────────────────────┐
│ Litestar version │ 2.0.0                │
│ Debug mode       │ Enabled              │
│ CORS             │ Disabled             │
│ CSRF             │ Disabled             │
│ OpenAPI          │ Enabled path=/schema │
│ Compression      │ Disabled             │
└──────────────────┴──────────────────────┘

Are you sure you don't have an __init__.py file inside your src directory?

provinzkraut commented 1 year ago

I upgraded to litestar-2.0.0a5

It's not yet released. You'll have to test this on the main branch or wait for the next alpha release (should happen soon).

patrickarmengol commented 1 year ago

I upgraded to litestar-2.0.0a5

It's not yet released. You'll have to test this on the main branch or wait for the next alpha release (should happen soon).

I think they were referring to my comment, not the most recent change involving --app-dir.

Are you sure you don't have an __init__.py file inside your src directory?

No __init__.py under src/. Here's my output with a barebones app:

(.venv) [iu@iroh calculon]$ tree
.
├── src
│   └── calculon
│       ├── app.py
│       └── __init__.py
└── pyproject.toml

3 directories, 3 files
(.venv) [iu@iroh calculon]$ litestar --app src.calculon.app:application info
Using Litestar app from env: 'src.calculon.app:application'
┌──────────────────┬──────────────────────┐
│ Litestar version │ 2.0.0                │
│ Debug mode       │ Disabled             │
│ CORS             │ Disabled             │
│ CSRF             │ Disabled             │
│ OpenAPI          │ Enabled path=/schema │
│ Compression      │ Disabled             │
└──────────────────┴──────────────────────┘

But then I saw what was actually causing the error in your example, from calculon import __version__. Since calculon is not a module recognized in your path, you get a ModuleNotFoundError. The same error occurs when running straight from uvicorn.

File "/home/iu/dev/calculon/src/calculon/app.py", line 2, in <module>
    from calculon import __version__
ModuleNotFoundError: No module named 'calculon'

So yes, a custom app directory would need to be specified in this case.

That being said, I think not installing the module into your environment kind of defeats the point of using the src directory layout altogether (see src-vs-flat and this blog post).

So what I do instead is pip install -e . and run litestar --app calculon.app:application.

Unfortunately, it seems a lot of the benefits of the src directory (prepping for publishing a package to PyPI) become moot for a web app, since you don't intend to publish the package. I'm guessing that's why a lot of python web app projects still use a flat layout.

pawelad commented 1 year ago

But then I saw what was actually causing the error in your example, from calculon import __version__. Since calculon is not a module recognized in your path, you get a ModuleNotFoundError. The same error occurs when running straight from uvicorn.

File "/home/iu/dev/calculon/src/calculon/app.py", line 2, in <module>
    from calculon import __version__
ModuleNotFoundError: No module named 'calculon'

So yes, a custom app directory would need to be specified in this case.

A very nice catch! You're absolutely right : )

That being said, I think not installing the module into your environment kind of defeats the point of using the src directory layout altogether (see src-vs-flat and this blog post).

So what I do instead is pip install -e . and run litestar --app calculon.app:application.

Unfortunately, it seems a lot of the benefits of the src directory (prepping for publishing a package to PyPI) become moot for a web app, since you don't intend to publish the package. I'm guessing that's why a lot of python web app projects still use a flat layout.

That's a very fair point, but I just irrationally love the src layout and how the directory conceptually meshes with docs, tests, etc. 😅

I'll give the articles though and maybe change my mind - thanks for linking them!

Also, I'd like to thank you all for the really thoughtful replies 😊