Skip to content
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

Fix: ASGI processes did not support lifespan events #295

Merged
merged 2 commits into from
Sep 25, 2023
Merged

Conversation

hoh
Copy link
Member

@hoh hoh commented Apr 20, 2023

ASGI specification https://asgi.readthedocs.io/en/latest/specs/lifespan.html

Fixes #293
Replaces #294

Missing

  • Fastapi example uses lifetimes
  • Tests validate that the startup lifetime is executed

@hoh hoh requested a review from MHHukiewitz April 20, 2023 15:04
@hoh hoh marked this pull request as draft April 20, 2023 15:11
Copy link
Member

@MHHukiewitz MHHukiewitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but would like to test it out locally tomorrow

vm_supervisor/__main__.py Outdated Show resolved Hide resolved
@hoh hoh changed the title Fix: ASGI processes did not support lifetime events Fix: ASGI processes did not support lifespan events Apr 20, 2023
@hoh
Copy link
Member Author

hoh commented Apr 20, 2023

Needs a fix in aleph-client to work.

@hoh
Copy link
Member Author

hoh commented Apr 20, 2023

Test by accessing https://aleph-vm-lab-2.aleph.cloud/vm/test/lifespan.

Commits squashed.

@hoh
Copy link
Member Author

hoh commented Apr 20, 2023

Tests fail due to the issue in aleph-client.

@MHHukiewitz MHHukiewitz marked this pull request as ready for review April 21, 2023 07:20
@MHHukiewitz
Copy link
Member

MHHukiewitz commented Apr 26, 2023

@hoh Is there a reason why you did not implement the same for the Debian runtime?

EDIT: Also, what's curious is that even though you did not change the Debian runtime, it also fails in the GH action (but I cannot really make out why that is, I'm still struggling with making the VM supervisor run on my machine due to Conda, duh)

@hoh
Copy link
Member Author

hoh commented Apr 26, 2023

@hoh Is there a reason why you did not implement the same for the Debian runtime?

I did. The init1.py of the Debian runtime is a symbolic link to the file I modified.

It also fails in the GH action (but I cannot really make out why that is, I'm still struggling with making the VM supervisor run on my machine due to Conda, duh)

The reason why the tests fail still need investigation.

@hoh hoh force-pushed the hoh-lifespan-events branch 4 times, most recently from ecd5874 to c68bce3 Compare May 16, 2023 14:09
@hoh
Copy link
Member Author

hoh commented Jul 5, 2023

This needs to be rebased due to recent changes.

@hoh hoh force-pushed the hoh-lifespan-events branch 5 times, most recently from 3589e00 to 90aabc7 Compare July 11, 2023 10:38
@MHHukiewitz
Copy link
Member

Any idea why the tests might be failing?

Copy link
Collaborator

@odesenfans odesenfans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of unrelated code, and the demo does not work.

runtimes/aleph-debian-11-python/init1.py Outdated Show resolved Hide resolved
runtimes/aleph-debian-11-python/init1.py Outdated Show resolved Hide resolved
@hoh hoh force-pushed the hoh-lifespan-events branch 2 times, most recently from 29881f4 to f585266 Compare September 25, 2023 14:53
Lifespan is a new feature that requires a new runtime to be deployed before it can be enforced.
@hoh hoh merged commit ea23354 into main Sep 25, 2023
13 checks passed
@MHHukiewitz MHHukiewitz deleted the hoh-lifespan-events branch September 25, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ASGI Lifespan Protocol needs to be implemented
3 participants