-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dispatch without FastAPI #154
Conversation
This will be great for onboarding 👍 |
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
6e14930
to
49da99e
Compare
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
try: | ||
if init is not None: | ||
init(*args, **kwargs) | ||
server.serve_forever() | ||
finally: | ||
server.shutdown() | ||
server.server_close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you Ctrl+C out of the blocking call? Should we catch KeyboardInterrupt
and pass
so that an exception isn't raised and stack trace printed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happens:
$ ./dispatch run -- ./main.py
╭───────────────────────────────────────────────────────────────────╮
│ │
│ Starting Dispatch session: ce77804d-b40d-4df7-b8a4-d10afcc4ab31 │
│ │
│ Run 'dispatch help run' to learn about Dispatch sessions. │
│ │
╰───────────────────────────────────────────────────────────────────╯
Hello, World!
127.0.0.1 - - [22/Apr/2024 18:22:23] "POST /dispatch.sdk.v1.FunctionService/Run HTTP/1.1" 200 -
^C
╭────────────────────────────────────────────────────────────────────────────────╮
│ │
│ To resume this Dispatch session: │
│ │
│ dispatch run --session ce77804d-b40d-4df7-b8a4-d10afcc4ab31 -- ./main.py │
│ │
╰────────────────────────────────────────────────────────────────────────────────╯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right because we send SIGTERM
rather than passing on the signal the CLI receives here: https://github.com/stealthrocket/dispatch/blob/00ad5441b8da130d378af1843af364241906b828/cli/run.go#L171. I think I had it passing on the signal originally, but uvicorn
was handling SIGINT
differently when it was run inside the CLI (and wasn't itself interactive) and it was printing a nasty stack trace.
Co-authored-by: Chris O'Hara <[email protected]>
Co-authored-by: Chris O'Hara <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
Signed-off-by: Achille Roussel <[email protected]>
This PR relates to dispatchrun/dispatch#12 and dispatchrun/dispatch#19, and is intended to make FastAPI an optional dependency for Dispatch.
I first went down the path of using the gRPC service as a replacement; however, requiring HTTP/2 made a few things difficult since the CLI will attempt HTTP/1 requests (because the connectrpc protocol allows it); see
dispatch.grpc
.Rather than hacking something with fake certs and whatnot, I added another implementation using
http.server
, which is very similar to the one with FastAPI but integrates with the standard library only; seedispatch.http
.This is still work in progress, I need to add tests.