Skip to content

Commit

Permalink
Expose Object.hydrate as public API / deprecate Object.resolve (#2769)
Browse files Browse the repository at this point in the history
* Expose Object.hydrate as public API / deprecate Object.resolve

* Add some basic test coverage

* Polish: add docstring, return self

* Fix ruff

* Downgrade .resolve deprecation to pending; we use it in integration tests

* Require synchronicity 0.9.9

* Fix docstring
  • Loading branch information
mwaskom authored Jan 22, 2025
1 parent 28b7db9 commit 12904f5
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 17 deletions.
35 changes: 27 additions & 8 deletions modal/_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
from google.protobuf.message import Message
from typing_extensions import Self

from modal._utils.async_utils import aclosing

from ._resolver import Resolver
from ._utils.async_utils import aclosing
from ._utils.deprecation import deprecation_warning
from .client import _Client
from .config import config, logger
from .exception import ExecutionError, InvalidError
Expand Down Expand Up @@ -238,31 +238,50 @@ def default_deps(*args, **kwargs) -> Sequence["_Object"]:

async def resolve(self, client: Optional[_Client] = None):
"""mdmd:hidden"""
obj = self.__class__.__name__.strip("_")
deprecation_warning(
(2025, 1, 16),
f"The `{obj}.resolve` method is deprecated and will be removed in a future release."
f" Please use `{obj}.hydrate()` or `await {obj}.hydrate.aio()` instead."
"\n\nNote that it is rarely necessary to explicitly hydrate objects, as most methods"
" will lazily hydrate when needed.",
show_source=False, # synchronicity interferes with attributing source correctly
pending=True,
)
await self.hydrate(client)

async def hydrate(self, client: Optional[_Client] = None) -> Self:
"""Synchronize the local object with its identity on the Modal server.
It is rarely necessary to call this method explicitly, as most operations
will lazily hydrate when needed. The main use case is when you need to
access object metadata, such as its ID.
"""
if self._is_hydrated:
# memory snapshots capture references which must be rehydrated
# on restore to handle staleness.
if self.client._snapshotted and not self._is_rehydrated:
# memory snapshots capture references which must be rehydrated
# on restore to handle staleness.
logger.debug(f"rehydrating {self} after snapshot")
self._is_hydrated = False # un-hydrate and re-resolve
c = client if client is not None else await _Client.from_env()
resolver = Resolver(c)
await resolver.load(typing.cast(_Object, self))
self._is_rehydrated = True
logger.debug(f"rehydrated {self} with client {id(c)}")
return
elif not self._hydrate_lazily:
# TODO(michael) can remove _hydrate lazily? I think all objects support it now?
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()
resolver = Resolver(c)
await resolver.load(self)
return self


def live_method(method):
@wraps(method)
async def wrapped(self, *args, **kwargs):
await self.resolve()
await self.hydrate()
return await method(self, *args, **kwargs)

return wrapped
Expand All @@ -271,7 +290,7 @@ async def wrapped(self, *args, **kwargs):
def live_method_gen(method):
@wraps(method)
async def wrapped(self, *args, **kwargs):
await self.resolve()
await self.hydrate()
async with aclosing(method(self, *args, **kwargs)) as stream:
async for item in stream:
yield item
Expand Down
3 changes: 2 additions & 1 deletion modal/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1207,12 +1207,13 @@ async def web_url(self) -> str:
async def is_generator(self) -> bool:
"""mdmd:hidden"""
# hacky: kind of like @live_method, but not hydrating if we have the value already from local source
# TODO(michael) use a common / lightweight method for handling unhydrated metadata properties
if self._is_generator is not None:
# this is set if the function or class is local
return self._is_generator

# not set - this is a from_name lookup - hydrate
await self.resolve()
await self.hydrate()
assert self._is_generator is not None # should be set now
return self._is_generator

Expand Down
3 changes: 2 additions & 1 deletion modal/sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,9 @@ async def exec(
raise InvalidError(f"workdir must be an absolute path, got: {workdir}")

# Force secret resolution so we can pass the secret IDs to the backend.
# TODO should we parallelize this?
for secret in secrets:
await secret.resolve(client=self._client)
await secret.hydrate(client=self._client)

task_id = await self._get_task_id()
req = api_pb2.ContainerExecRequest(
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ install_requires =
grpclib==0.4.7
protobuf>=3.19,<6.0,!=4.24.0
rich>=12.0.0
synchronicity~=0.9.8
synchronicity~=0.9.9
toml
typer>=0.9
types-certifi
Expand Down
2 changes: 1 addition & 1 deletion test/cls_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def test_lookup(client, servicer):
assert obj.bar.local(1, 2)


def test_from_name_lazy_method_resolve(client, servicer):
def test_from_name_lazy_method_hydration(client, servicer):
deploy_app(app, "my-cls-app", client=client)
cls: Cls = Cls.from_name("my-cls-app", "Foo")

Expand Down
6 changes: 3 additions & 3 deletions test/container_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2242,10 +2242,10 @@ def test_class_as_service_serialized(servicer):


@skip_github_non_linux
def test_function_lazy_resolution(servicer, credentials, set_env_client):
def test_function_lazy_hydration(servicer, credentials, set_env_client):
# Deploy some global objects
Volume.from_name("my-vol", create_if_missing=True).resolve()
Queue.from_name("my-queue", create_if_missing=True).resolve()
Volume.from_name("my-vol", create_if_missing=True).hydrate()
Queue.from_name("my-queue", create_if_missing=True).hydrate()

# Run container
deploy_app_externally(servicer, credentials, "test.supports.lazy_hydration", "app", capture_output=False)
Expand Down
17 changes: 15 additions & 2 deletions test/object_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

from modal import Secret
from modal._object import _Object
from modal.dict import _Dict
from modal.exception import InvalidError
from modal.dict import Dict, _Dict
from modal.exception import InvalidError, PendingDeprecationError
from modal.queue import _Queue


Expand All @@ -22,6 +22,19 @@ def test_new_hydrated(client):
_Object._new_hydrated("xy-123", client, None)


def test_on_demand_hydration(client):
obj = Dict.from_name("test-dict", create_if_missing=True).hydrate(client)
assert obj.object_id is not None


def test_resolve_deprecation(client):
obj = Dict.from_name("test-dict", create_if_missing=True)
warning = r"Please use `Dict.hydrate\(\)` or `await Dict.hydrate.aio\(\)`"
with pytest.warns(PendingDeprecationError, match=warning):
obj.resolve(client)
assert obj.object_id is not None


def test_constructor():
with pytest.raises(InvalidError) as excinfo:
Secret({"foo": 123})
Expand Down

0 comments on commit 12904f5

Please sign in to comment.