piccolo-orm / piccolo_admin

A powerful web admin for your database.
https://piccolo-orm.com/ecosystem/
MIT License
299 stars 35 forks source link

Is here a granian support? #399

Open Forceres opened 6 days ago

Forceres commented 6 days ago

Does piccolo admin support granian instead of uvicorn? Piccolo CLI gives only 2 options: uvicorn and hypercorn

dantownsend commented 6 days ago

I haven't tried granian, but it should work - it supports ASGI apps.

We could add it as an option to piccolo asgi new. We would have to copy this code:

https://github.com/emmett-framework/granian/blob/de03e2c3f5f81b3372cac788bcff22d4471c1656/granian/cli.py#L251-L298

To here:

https://github.com/piccolo-orm/piccolo/blob/master/piccolo/apps/asgi/commands/templates/app/main.py.jinja

sinisaos commented 4 days ago

@dantownsend I just tried granian and the settings are pretty similar to uvicorn. We just need to add this to main.py.jinja

if __name__ == "__main__":
    ... # previous code
    {% elif server == 'granian' %}
    import granian

    granian.Granian("app:app", interface="asgi", reload=True).serve()
    {% endif %}

and this in new.py

SERVERS = ["uvicorn", "Hypercorn", "granian"]

and everything works.

dantownsend commented 4 days ago

@sinisaos Interesting - that's much simpler than I imagined. If you have time, can you create a PR and I'll merge it in?

sinisaos commented 4 days ago

@dantownsend PR is not a problem, but something is strange to me. The suggested code works in an old (not fresh) env where uvicorn is already used (and I just replace the uvicorn code with granian suggested code in main.py). But if I use a fresh env (or clear browser cache), Piccolo Admin doesn't work (FastAPI docs do). Then I found this issue with Starlette that describes the problem. I won't be doing PR for now. I'm sorry I didn't check it before, but I wasn't aware of that problem because everything worked in the old env when uvicorn was first used. Sorry again.

dantownsend commented 4 days ago

@sinisaos No worries. I wasn't aware of the path send extension, but here are some docs for reference:

https://asgi.readthedocs.io/en/latest/extensions.html#path-send

sinisaos commented 4 days ago

@dantownsend Just for info, I tried patching Starlette with this PR (from the author of granian) and everything works. If that PR would merge, we can add granian support with the code suggested earlier.

dantownsend commented 4 days ago

@sinisaos Thanks - that PR looks close to completion, so fingers crossed.