Skip to content

Commit

Permalink
PoC: get rid of hydrate_lazily concept
Browse files Browse the repository at this point in the history
  • Loading branch information
mwaskom committed Jan 16, 2025
1 parent 1761b00 commit 0ec0e9f
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 39 deletions.
10 changes: 2 additions & 8 deletions modal/cls.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,7 @@ def _deps():

rep = f"Method({method_name})"

fun = _Function._from_loader(
_load,
rep,
deps=_deps,
hydrate_lazily=True,
)
fun = _Function._from_loader(_load, rep, deps=_deps)
if service_function.is_hydrated:
# Eager hydration (skip load) if the instance service function is already loaded
hydrate_from_instance_service_function(fun)
Expand Down Expand Up @@ -350,7 +345,6 @@ async def method_loader(fun, resolver: Resolver, existing_object_id):
method_loader,
repr,
deps=lambda: [], # TODO: use cls as dep instead of loading inside method_loader?
hydrate_lazily=True,
)


Expand Down Expand Up @@ -594,7 +588,7 @@ async def _load_remote(obj: _Object, resolver: Resolver, existing_object_id: Opt
obj._hydrate(response.class_id, resolver.client, response.handle_metadata)

rep = f"Ref({app_name})"
cls = cls._from_loader(_load_remote, rep, is_another_app=True, hydrate_lazily=True)
cls = cls._from_loader(_load_remote, rep, is_another_app=True)
# TODO: when pre 0.63 is phased out, we can set class_service_function here instead
cls._name = name
return cls
Expand Down
2 changes: 1 addition & 1 deletion modal/dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async def _load(self: _Dict, resolver: Resolver, existing_object_id: Optional[st
logger.debug(f"Created dict with id {response.dict_id}")
self._hydrate(response.dict_id, resolver.client, None)

return _Dict._from_loader(_load, "Dict()", is_another_app=True, hydrate_lazily=True)
return _Dict._from_loader(_load, "Dict()", is_another_app=True)

@staticmethod
@renamed_parameter((2024, 12, 18), "label", "name")
Expand Down
2 changes: 1 addition & 1 deletion modal/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ async def _load(self: _Environment, resolver: Resolver, existing_object_id: Opti
self._hydrate(response.environment_id, resolver.client, response.metadata)

# TODO environment name (and id?) in the repr? (We should make reprs consistently more useful)
return _Environment._from_loader(_load, "Environment()", is_another_app=True, hydrate_lazily=True)
return _Environment._from_loader(_load, "Environment()", is_another_app=True)

@staticmethod
@renamed_parameter((2024, 12, 18), "label", "name")
Expand Down
4 changes: 2 additions & 2 deletions modal/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -995,7 +995,7 @@ async def _load(param_bound_func: _Function, resolver: Resolver, existing_object
response = await retry_transient_errors(parent._client.stub.FunctionBindParams, req)
param_bound_func._hydrate(response.bound_function_id, parent._client, response.handle_metadata)

fun: _Function = _Function._from_loader(_load, "Function(parametrized)", hydrate_lazily=True)
fun: _Function = _Function._from_loader(_load, "Function(parametrized)")

if can_use_parent and parent.is_hydrated:
# skip the resolver altogether:
Expand Down Expand Up @@ -1079,7 +1079,7 @@ async def _load_remote(self: _Function, resolver: Resolver, existing_object_id:
self._hydrate(response.function_id, resolver.client, response.handle_metadata)

rep = f"Ref({app_name})"
return cls._from_loader(_load_remote, rep, is_another_app=True, hydrate_lazily=True)
return cls._from_loader(_load_remote, rep, is_another_app=True)

@staticmethod
@renamed_parameter((2024, 12, 18), "tag", "name")
Expand Down
2 changes: 1 addition & 1 deletion modal/network_file_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ async def _load(self: _NetworkFileSystem, resolver: Resolver, existing_object_id
)
raise

return _NetworkFileSystem._from_loader(_load, "NetworkFileSystem()", hydrate_lazily=True)
return _NetworkFileSystem._from_loader(_load, "NetworkFileSystem()")

@classmethod
@asynccontextmanager
Expand Down
22 changes: 1 addition & 21 deletions modal/object.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class _Object:
_preload: Optional[Callable[[O, Resolver, Optional[str]], Awaitable[None]]]
_rep: str
_is_another_app: bool
_hydrate_lazily: bool
_deps: Optional[Callable[..., list["_Object"]]]
_deduplication_key: Optional[Callable[[], Awaitable[Hashable]]] = None

Expand All @@ -65,7 +64,6 @@ def _init(
load: Optional[Callable[[O, Resolver, Optional[str]], Awaitable[None]]] = None,
is_another_app: bool = False,
preload: Optional[Callable[[O, Resolver, Optional[str]], Awaitable[None]]] = None,
hydrate_lazily: bool = False,
deps: Optional[Callable[..., list["_Object"]]] = None,
deduplication_key: Optional[Callable[[], Awaitable[Hashable]]] = None,
):
Expand All @@ -74,7 +72,6 @@ def _init(
self._preload = preload
self._rep = rep
self._is_another_app = is_another_app
self._hydrate_lazily = hydrate_lazily
self._deps = deps
self._deduplication_key = deduplication_key

Expand Down Expand Up @@ -123,24 +120,10 @@ def _get_metadata(self) -> Optional[Message]:
# the object_id is already provided by other means
return

def _validate_is_hydrated(self: O):
if not self._is_hydrated:
object_type = self.__class__.__name__.strip("_")
if hasattr(self, "_app") and getattr(self._app, "_running_app", "") is None:
# The most common cause of this error: e.g., user called a Function without using App.run()
reason = ", because the App it is defined on is not running"
else:
# Technically possible, but with an ambiguous cause.
reason = ""
raise ExecutionError(
f"{object_type} has not been hydrated with the metadata it needs to run on Modal{reason}."
)

def clone(self: O) -> O:
"""mdmd:hidden Clone a given hydrated object."""

# Object to clone must already be hydrated, otherwise from_loader is more suitable.
self._validate_is_hydrated()
obj = type(self).__new__(type(self))
obj._initialize_from_other(self)
return obj
Expand All @@ -152,13 +135,12 @@ def _from_loader(
rep: str,
is_another_app: bool = False,
preload: Optional[Callable[[O, Resolver, Optional[str]], Awaitable[None]]] = None,
hydrate_lazily: bool = False,
deps: Optional[Callable[..., Sequence["_Object"]]] = None,
deduplication_key: Optional[Callable[[], Awaitable[Hashable]]] = None,
):
# TODO(erikbern): flip the order of the two first arguments
obj = _Object.__new__(cls)
obj._init(rep, load, is_another_app, preload, hydrate_lazily, deps, deduplication_key)
obj._init(rep, load, is_another_app, preload, deps, deduplication_key)
return obj

@classmethod
Expand Down Expand Up @@ -236,8 +218,6 @@ async def resolve(self, client: Optional[_Client] = None):
self._is_rehydrated = True
logger.debug(f"rehydrated {self} with client {id(c)}")
return
elif not self._hydrate_lazily:
self._validate_is_hydrated()
else:
# TODO: this client and/or resolver can't be changed by a caller to X.from_name()
c = client if client is not None else await _Client.from_env()
Expand Down
2 changes: 1 addition & 1 deletion modal/queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ async def _load(self: _Queue, resolver: Resolver, existing_object_id: Optional[s
response = await resolver.client.stub.QueueGetOrCreate(req)
self._hydrate(response.queue_id, resolver.client, None)

return _Queue._from_loader(_load, "Queue()", is_another_app=True, hydrate_lazily=True)
return _Queue._from_loader(_load, "Queue()", is_another_app=True)

@staticmethod
@renamed_parameter((2024, 12, 18), "label", "name")
Expand Down
6 changes: 3 additions & 3 deletions modal/secret.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ async def _load(self: _Secret, resolver: Resolver, existing_object_id: Optional[
self._hydrate(resp.secret_id, resolver.client, None)

rep = f"Secret.from_dict([{', '.join(env_dict.keys())}])"
return _Secret._from_loader(_load, rep, hydrate_lazily=True)
return _Secret._from_loader(_load, rep)

@staticmethod
def from_local_environ(
Expand Down Expand Up @@ -159,7 +159,7 @@ async def _load(self: _Secret, resolver: Resolver, existing_object_id: Optional[

self._hydrate(resp.secret_id, resolver.client, None)

return _Secret._from_loader(_load, "Secret.from_dotenv()", hydrate_lazily=True)
return _Secret._from_loader(_load, "Secret.from_dotenv()")

@staticmethod
@renamed_parameter((2024, 12, 18), "label", "name")
Expand Down Expand Up @@ -202,7 +202,7 @@ async def _load(self: _Secret, resolver: Resolver, existing_object_id: Optional[
raise
self._hydrate(response.secret_id, resolver.client, None)

return _Secret._from_loader(_load, "Secret()", hydrate_lazily=True)
return _Secret._from_loader(_load, "Secret()")

@staticmethod
@renamed_parameter((2024, 12, 18), "label", "name")
Expand Down
2 changes: 1 addition & 1 deletion modal/volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ async def _load(self: _Volume, resolver: Resolver, existing_object_id: Optional[
response = await resolver.client.stub.VolumeGetOrCreate(req)
self._hydrate(response.volume_id, resolver.client, None)

return _Volume._from_loader(_load, "Volume()", hydrate_lazily=True)
return _Volume._from_loader(_load, "Volume()")

@classmethod
@asynccontextmanager
Expand Down

0 comments on commit 0ec0e9f

Please sign in to comment.