Make all "constructors" take a client optional arg

[freider]

Goals of this patch:

• Allow all .from_name(), .from_id() etc. user-facing "constructors" to take an optional client object
• Get rid of all set_env_client fixtures in tests - all test code should be functioning by passing in Client objects explicitly through various means

Closes SDK-632

Why this is hard

• Cls and Function can either be hydrated as part of an app.deploy(client=...)/run(client=...) or lazily
• In the deploy hydration case ("non-lazy"), the client isn't actually available when the Function object is created, since the App isn't "live" at decoration time
• There are various ways in which we create lazy "clones" of object that should "inherit" their original's client, e.g. Cls.with_options(). This clone can also be created before client has been set on its parent, and once the client is set on the parent it should transfer to the clones as well 😵
• Objects may initialize "load" of independent other objects when they are themselves loaded, in which case they should transfer their own Client (and environment etc.) to the independent "dependency" (e.g. a Sandbox.create() with a set of Secrets as deps.
• If a "dependency" lacks a specific client, it should use its parent's Client. If it gets a client from its parent it should not create a from_env client unnecessarily

Bonus goal

• The same rules for client above applies to environment_name as well where it's applicable, and to some degree app_id - these are also load-time metadata that can either be explicitly injected or inferred from parents or the runtime.

Change summary

• Add a new LoadContext type and concept, that encapsulates optional values of client, environment_name and app_id that were previously part of the Resolver
• Remove the client, environment_name and app_id fields from Resolver
• Add optional overrides of all LoadContext vars into modal._Objects via a object._load_context field on unhydrated construction (typically from provided client and environment_name values in .from_* constructors.
• App also gets an internal LoadContext (initially empty on App() construction)
• The signature of the _load() loader method on _Object now receives both a Resolver and a LoadContext object and load-code should use load_context.client etc. for rpcs and to set the _Object._client when hydrating.
  The new "loader function" signature is:

def _load(instance: _Object, resolver: Resolver, load_context: LoadContext, existing_object_id: Optional[str]) -> None

The LoadContext passed is guaranteed to contain a non-optional Client and environment_name, and an app_id if one exists.

• The Resolver instance that is now passed to loaders only serves to load other instances explicitly via .load(other_obj). Resolver.load() call will now take both an object and a "parent LoadContext". The Resolver then "merges" into each loaded Object's passed parent load context with whatever the object has optionally specified as overrides in self._load_context. This makes sure that we prioritize the context overrides of each object, but allow the "parent" context to propagate to any child that doesn't have overrides. Most of the dependency loading is still handled automatically via "deps" passed to the Object._from_loader() contstructor.

Slightly hacky: LoadContext sharing for Cls/Function "clones"

While normally we give each object its own LoadContext overrides instance, in the case of Cls (and a future Function.with_options()) we need to make sure that all related entities share the same LoadContext reference.

• This is because the "load dag" of Cls->Obj->Method is "inverted" from a load context perspective - the optional client would be injected at Cls.from_name(client=...), but the "leaf" _Object (a Function representing a method on the class) has no such override mechanims in itself. The method Function is however top level/"root" object when resolving dependencies during lazy loading, so in order for it to get the
• By making a Cls reuse its load context from the Cls itself when creating its methods, we make sure that whatever client specified as an override on Cls is also used when loading the method Function.
• In addition we also make any "app creation" Functions and Cls use a reference to an app associated LoadContext, which means this load context gets referenced by all Functions, Cls, Obj and Methods from that app. We then make sure app.run updates this load context in place, effectively propagating any load_context overrides from app launch to the lazy-loaded "Method" objects that might get referenced by users. This fixes the messy case of:

app = App()
@app.cls()
class F:
     @method
     def foo(self): ...

f = F()

with app.run(client=SomeClient):
    

Compatibility checklist

Check these boxes or delete any item (or this section) if not relevant for this PR.

• Client+Server: this change is compatible with old servers
• Client forward compatibility: this change ensures client can accept data intended for later versions of itself

Note on protobuf: protobuf message changes in one place may have impact to
multiple entities (client, server, worker, database). See points above.

---

Release checklist

If you intend for this commit to trigger a full release to PyPI, please ensure that the following steps have been taken:

• Version file (modal_version/__init__.py) has been updated with the next logical version
• Changelog has been cleaned up and given an appropriate subhead

---

Changelog

---

Note

Add LoadContext and make all from_* constructors accept an optional client, moving client/env/app_id from Resolver to a propagated load context.

• Core API:
  • Introduce LoadContext (client, environment_name, app_id) and propagate through all object loads.
  • Remove client/env/app_id from Resolver; it now merges parent and per-object load contexts and applies defaults.
• Object/Resolver changes:
  • Update _Object._from_loader/hydrate/loader signatures to accept LoadContext; hydrate uses Resolver.load(self, parent_load_context).
  • Dedup/hydration logic now hydrates using load_context.client.
• App/Runner:
  • App holds a mutable _load_context; runner sets client/env/app_id and passes it to Resolver for all loads.
• Constructors:
  • Function, Cls, Dict, Queue, Environment, Image, Mount, NetworkFileSystem, Proxy, Secret, Volume, Sandbox, SandboxSnapshot, FunctionCall updated to use LoadContext and accept client (and env) in from_name/from_id.
  • Ensure method/param-bound functions and Cls.with_* share parent LoadContext by reference.
• CLI:
  • Commands construct a LoadContext (client/env) and call Resolver.load(...) without embedding client.
• Tests:
  • Remove set_env_client usage; pass client explicitly; update expectations accordingly.

^{Written by Cursor Bugbot for commit fe10c23. This will update automatically on new commits. Configure here.}

[freider]

One of the few places I had to keep the cursed env_client injections. It's a bit of an odd case and I have some ideas for how to fix it if really want to, but shouldn't be a huge deal since it's only for a particular error case

[mwaskom]

Why is this "parent metadata"? What's the parent here?

[mwaskom]

BTW feels like we could be using modal.Queue.objects.create() here?

[mwaskom]

😵‍💫

[mwaskom]

Why do we have a "private" parameter to this (already private?) object

[mwaskom]

Hello

[thomasjpfan]

Does _preload need the resolve anymore?

[freider]

Probably not in practice, but technically something that preloads should be able to preload other objects

[thomasjpfan]

For consistency?

Suggested change

	self._load_context = LoadContext() # Initialize empty LoadContext
	self._load_context = LoadContext.empty() # Initialize empty LoadContext

[freider]

👍

[thomasjpfan]

At a glance, it looks like LoadContext is used as a "builder" to combine contexts. Can we have LoadContext.apply_defaults return a ResolvedContext, where the parameters are not None:

@dataclass
class ResolvedContext:
   client: _Client
   environment_name: str
   app_id: Optiona[str]

I would even rename LoadContext into ContextBuilder. With ResolvedContext, we can be sure that the client is defined with the type system.

[freider]

Yeah that could be nice!

One argument against would be if we think ahead to when the Resolver doesn't exist anymore and all contexts are just propagated from their "parents" explicitly (instead of using deps). In that scenario, I don't think we want to apply_default() anymore, and instead just apply each property as it's needed from within the loader (if it's not already set on the load_context_overrides.

Making each property resolve individually would fix the case where we have loaders that may raise exceptions immediately without actually needing a functional context (like calling methods directly on un-instantiated classes). Maybe we should actually already make that change, e.g. having LoadContext have async def get_client() -> Client etc. so that loaders use those (even though we currently have to make the resolver apply the defaults first due to deps...)

[thomasjpfan]

Nit:

Suggested change

	parent_metadata = LoadContext(client=client)
	parent_context = LoadContext(client=client)

[freider]

Nice catch! I initially called the object LoadMetadata, but must have missed this rename

[thomasjpfan]

Suggested change

	parent_metadata = LoadContext(client=client)
	parent_context = LoadContext(client=client)