Conversation
| username = f'service-{service_name}-{uuid.uuid4()}' | ||
| user = await hub.ensure_user(username, create_user=True) | ||
| try: | ||
| if user['server'] is None: |
There was a problem hiding this comment.
There was a problem hiding this comment.
The reason why I didn't use hub.ensure_server is because I wanted to call hub.ensure_user myself (first thing that ensure_server does is to create the user) so that if the task cancellation happens after the user creation, I know I should delete the user as part of the cleanup phase. Do you think it makes sense?
| # user's server if the request got to it. Let's make sure there's no server running for the user. | ||
| # Otherwise, users' single-servers will continue to run and hub will return | ||
| # "SingleUserNotebookApp mixins:520] Error notifying Hub of activity" | ||
| resp = await hub.session.delete(hub.api_url / 'users' / username / 'server') |
There was a problem hiding this comment.
Can hub.delete_server and a hub.delete_user be used here?
There was a problem hiding this comment.
We are actually using hub.delete_user at line 62.
The reason why hub.delete_server is not used here too is because of a behavior I noticed:
The hub.create_server function returns after it has asked the hub to create the server. But this doesn't mean that the single user server is in a ready state (It could be in a pending state). If task cancellation happens before the single user server is ready, a hub.delete_server call will get a a 40X response code (without actually returning this to the caller function) and the single-user server won't get deleted.
So, we could potentially end up with lots of running user servers.
|
Two comments. Other than that, this seems like a thing we should try out! \o/ |
|
I'll make a series of PRs to this PR, to make life easier for review :) |
2 participants
Should be eventually moved to its own repo.
Here for CR only.