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

Illustrate create() classmethod idea #3801

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

athornton
Copy link
Contributor

This is a draft PR meant to illustrate the "initialize async things at instance creation time" approach I am talking about with respect to jupyterhub/kubespawner#563

@consideRatio
Copy link
Member

I inspected how spawner objects are initialized, and it was tricky to switch to awaiting an async constructor. The reason is that they are lazily created objects - based on when the property spawners or spawner of a User object is used.

Each User object gets a _SpawnerDIct object initialized to the User objects spawners property.

class _SpawnerDict(dict):
def __init__(self, spawner_factory):
self.spawner_factory = spawner_factory
def __getitem__(self, key):
if key not in self:
self[key] = self.spawner_factory(key)
return super().__getitem__(key)

Then, this will create the instances when needed.

@minrk
Copy link
Member

minrk commented Feb 22, 2022

Yeah, Spawners are accessed in a synchronous way in a lot of places, and indeed making access of spawners async will lead to race conditions if we aren't careful (we must guarantee that we can't start another async creation of the same spawner name while awaiting the first one). We'll need to at the very least ensure we initialize things like:

  • presence in the .spawners dict
  • pending/etc. state

before any awaits happen. So it will have to be two-stage somewhere, whether that's JupyterHub putting a placeholder Future in the dict

What might be easier is to officially support two-stage:

obj = Class(**)
if hasattr(obj, 'initialize_async'):
    await obj.initialize_async()

Across Jupyter/traitlets we have a the de facto pattern of an .initialize() method used to finish initialization of an instance, mostly in Application classes, which is async in JupyterHub. We could follow that pattern.

One way to support async init would be to replace the spawners dict with a dict of futures, so all the user.spawners[name] would become await user.spawners[name]. In this way, the item-access on the dict is actually synchronous, creating the Future (should resolve races), while the Future itself is awaiting the creation. The downside is that this adds a lot of awaits in a lot of places for objects that will usually be ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants