emmett-framework / granian

A Rust HTTP server for Python applications
BSD 3-Clause "New" or "Revised" License
2.67k stars 79 forks source link

ASGI Lifespan for Django #297

Closed Andrew-Chen-Wang closed 4 months ago

Andrew-Chen-Wang commented 4 months ago

Hi I'm getting the following with a standard Django app OOTB (version 5.0.6, granian 1.3.2)

granian web.config.asgi:application --interface asgi --reload

[INFO] Starting granian (main PID: 31553)
[INFO] Listening at: http://127.0.0.1:8000
[INFO] Spawning worker-1 with pid: 31555
[WARNING] ASGI Lifespan errored, continuing without Lifespan support (to avoid Lifespan completely use "asginl" interface)
[INFO] Started worker-1
[INFO] Started worker-1 runtime-1

To reproduce

python -m virtualenv venv
source venv/bin/activate
pip install django granian
django-admin startproject project
cd project
granian project.asgi:application --interface asgi
Andrew-Chen-Wang commented 4 months ago

Ah I see from the resolution of #153 that Django doesn't support it; I feel like the logger.warn should actually print out exc, at least during debugging log level

baseplate-admin commented 4 months ago

Perhaps a better alternative here is to make a documentation and add

"How to use Granian with django?"

gi0baro commented 4 months ago

I honestly think the current implementation is fine. Lifespan is a standard ASGI specification, the fact Django is widely adopted out there doesn't imply Granian should make exceptions in its handlers.

Eventually, the warning message might be enhanced to suggest the user to switch to asginl interface, but that's how much edits I am willing to do on the matter, also given the long discussions that took place in #153.

Andrew-Chen-Wang commented 4 months ago

@gi0baro why not at least pass the underlying exception as a logger debug message?

gi0baro commented 4 months ago

@gi0baro why not at least pass the underlying exception as a logger debug message?

It can be re-implemented. It was removed due to people complaining about receiving errors while the application was actually running fine and/or the amount of errors logged 🤷‍♂️

Andrew-Chen-Wang commented 4 months ago

I can understand why:

https://github.com/emmett-framework/granian/commit/08803c504dc04891582f82c7f125c1979c76f9d9

It seems like the exception was passed in for logger.warn, but most people's Sentry or loggers aren't set to DEBUG log level. Would it make sense to have two logging lines?:

except Exception as e:
    logger.warn("ASGI Lifespan protocol not supported")
    logger.debug("Exception", exc_info=e)
gi0baro commented 4 months ago

I can understand why:

08803c5

It seems like the exception was passed in for logger.warn, but most people's Sentry or loggers aren't set to DEBUG log level. Would it make sense to have two logging lines?:

except Exception as e:
    logger.warn("ASGI Lifespan protocol not supported")
    logger.debug("Exception", exc_info=e)

That would require to set --log-level debug in Granian though, even if Django is in debug mode.. To me seems fine, but I'm not sure it would be valuable to users.