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

Add App.from_name for construction of a lazily-hydratable App #2798

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 52 additions & 15 deletions modal/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import inspect
import typing
import warnings
from collections.abc import AsyncGenerator, Coroutine, Sequence
from collections.abc import AsyncGenerator, Awaitable, Coroutine, Sequence
from pathlib import PurePosixPath
from textwrap import dedent
from typing import (
Expand All @@ -17,6 +17,7 @@
import typing_extensions
from google.protobuf.message import Message
from synchronicity.async_wrap import asynccontextmanager
from typing_extensions import Self

from modal_proto import api_pb2

Expand Down Expand Up @@ -187,6 +188,9 @@ def foo():
_running_app: Optional[RunningApp] # Various app info
_client: Optional[_Client]

# For when the App is constructed with .from_name
_load: Optional[Callable[[Self, _Client], Awaitable[None]]] = None

def __init__(
self,
name: Optional[str] = None,
Expand Down Expand Up @@ -258,6 +262,44 @@ def description(self) -> Optional[str]:
"""The App's `name`, if available, or a fallback descriptive identifier."""
return self._description

@staticmethod
def from_name(name: str, environment_name: str = "", create_if_missing: bool = False) -> "_App":
"""Look up an App with a given name, creating a new App if necessary.

Note that Apps created through this method will be in a deployed state,
but they will not have any associated Functions or Classes.

This method is mainly useful for creating an App to associate with a Sandbox:

```python
app = modal.App.from_name("my-app", create_if_missing=True)
app.hydrate() # Pre-hydrate to accelerate Sandbox creation
modal.Sandbox.create("echo", "hi", app=app)
```

When building an App that will be populated with Functions and Classes, you should
call the main `App()` constructor directly.
"""

# TODO We need to enforce rules about App names, but we previously weren't in App.lookup
# So we probably need to add that gradually (i.e. as a warning first) to avoid breaking people?
async def _load(self: _App, client: _Client):
request = api_pb2.AppGetOrCreateRequest(
app_name=name,
environment_name=_get_environment_name(environment_name),
object_creation_type=(api_pb2.OBJECT_CREATION_TYPE_CREATE_IF_MISSING if create_if_missing else None),
)
response = await retry_transient_errors(client.stub.AppGetOrCreate, request)
self._app_id = response.app_id
self._client = client
# TODO should we also be calling AppGetLayout and populating the running_app with it?
self._running_app = RunningApp(response.app_id, interactive=False)

app = _App(name)
app._load = _load
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super minor but this could maybe just be a constructor argument

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m fairly averse to “private parameters” in public interfaces: best case, you confuse users or at least present them with information they don’t need to think about, worse case they try to use it and end up in trouble somehow.

Copy link
Contributor Author

@mwaskom mwaskom Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d really prefer to just merge __init__ and .from_name but couldn’t see a simple way to do that.


return app

@staticmethod
@renamed_parameter((2024, 12, 18), "label", "name")
async def lookup(
Expand All @@ -280,22 +322,17 @@ async def lookup(
if client is None:
client = await _Client.from_env()

environment_name = _get_environment_name(environment_name)

request = api_pb2.AppGetOrCreateRequest(
app_name=name,
environment_name=environment_name,
object_creation_type=(api_pb2.OBJECT_CREATION_TYPE_CREATE_IF_MISSING if create_if_missing else None),
)

response = await retry_transient_errors(client.stub.AppGetOrCreate, request)

app = _App(name)
app._app_id = response.app_id
app._client = client
app._running_app = RunningApp(response.app_id, interactive=False)
app = _App.from_name(name, environment_name=environment_name, create_if_missing=create_if_missing)
assert app._load is not None # Populated by from_name
await app._load(app, client)
return app

async def hydrate(self, client: Optional[_Client] = None):
if self._load is None:
raise InvalidError("Apps must be constructed with .from_name() to be lazily hydrated.")
await self._load(self, client if client is not None else await _Client.from_env())
return self

def set_description(self, description: str):
self._description = description

Expand Down
20 changes: 20 additions & 0 deletions test/app_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,3 +479,23 @@ def func(): # noqa: F811

app_3.include(app_4)
assert "Overriding existing function" in caplog.messages[0]


def test_from_name(servicer, client):
app = App.from_name(name := "lazy-app")
assert app.app_id is None
assert app.name == name
app.hydrate(client)
assert app.app_id


def test_lookup(servicer, client):
app = App.lookup(name := "eager-app", client=client)
assert app.app_id
assert app.name == name


def test_lazy_hydration(servicer, client):
app = App("unhydratable-app")
with pytest.raises(InvalidError, match=r"Apps must be constructed with .from_name\(\) to be lazily hydrated"):
app.hydrate(client)
Loading