Adapt bind_socket to make it usable with multiple processes #1009
Conversation
|
still WIP I noticed the uvicorn logger message needs to be adapted |
|
ready now @Kludex ! |
|
ready for another review @Kludex |
uvicorn/main.py
Outdated
| if config.uds: | ||
| os.remove(config.uds) |
There was a problem hiding this comment.
Maybe I'm mistaken, but can't we have a single one of those outside the conditional?
| if config.uds: | |
| os.remove(config.uds) | |
| if config.uds: | |
| os.remove(config.uds) |
There was a problem hiding this comment.
I guess we can, but it's not right, because we bind inside them. 🤔 Right?
There was a problem hiding this comment.
it looks simpler yes, I removed occurrences in the basereload and multiprocess shutdown method and put it outside
the one on config.py ? |
|
Yeah |
that's a good idea, he put a lot of efforts into it and it would be cruel to have to add that part on top of it ;) |
| @@ -395,37 +395,63 @@ def setup_event_loop(self) -> None: | |||
| loop_setup() | |||
|
|
|||
| def bind_socket(self) -> socket.socket: | |||
| family = socket.AF_INET | |||
| addr_format = "%s://%s:%d" | |||
| logger_args: List[Union[str, int]] | |||
There was a problem hiding this comment.
just needed to add this post-merge with master @Kludex
the rest stayed the same
There was a problem hiding this comment.
So I removed the hold button and feel free to merge if still ok with this !
There was a problem hiding this comment.
I find it so weird that the approval is not removed after a commit is pushed. I don't understand the reason behind it.
Anyway, LGTM! :)
* Adapt bind_socket to make it usable with multple processes * Lint * we are in single proc here * Remove socket file also in reloader * Tests * Tests * Remove socket on close * Changed if / else to check uds / fds * Refactored logging inside bind_socket * Added test for fd * Refactored removal of socket * Minimized diff post merge
) * Adapt bind_socket to make it usable with multple processes * Lint * we are in single proc here * Remove socket file also in reloader * Tests * Tests * Remove socket on close * Changed if / else to check uds / fds * Refactored logging inside bind_socket * Added test for fd * Refactored removal of socket * Minimized diff post merge
This makes it possible to use reload and workers flag with uds and fd.
fixes #428 #368 #586 #722