Skip to content

Commit

Permalink
fix merge conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
devennavani committed Nov 26, 2024
2 parents 1b4d2f0 + 457a51a commit 19e63fc
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 45 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ jobs:
run: pytest -v

- name: Run docstring tests
if: github.event.pull_request.head.repo.fork == 'false'
if: github.event.pull_request.head.repo.fork == false
env:
MODAL_ENVIRONMENT: client-doc-tests
MODAL_TOKEN_ID: ${{ secrets.MODAL_TOKEN_ID }}
Expand Down Expand Up @@ -243,4 +243,4 @@ jobs:
env:
TWINE_USERNAME: __token__
TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}
run: twine upload dist/* --non-interactive
run: twine upload dist/* --non-interactive
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ We appreciate your patience while we speedily work towards a stable release of t

<!-- NEW CONTENT GENERATED BELOW. PLEASE PRESERVE THIS COMMENT. -->

### 0.66.45 (2024-11-26)

- The `modal launch` CLI now accepts a `--detach` flag to run the App in detached mode, such that it will persist after the local client disconnects.



### 0.66.40 (2024-11-23)

* Adds `Image.add_local_file(..., copy=False)` and `Image.add_local_dir(..., copy=False)` as a unified replacement for the old `Image.copy_local_*()` and `Mount.add_local_*` methods.
Expand Down
63 changes: 39 additions & 24 deletions modal/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ def foo():

_name: Optional[str]
_description: Optional[str]
_indexed_objects: Dict[str, _Object]
_functions: Dict[str, _Function]
_classes: Dict[str, _Cls]

_image: Optional[_Image]
_mounts: Sequence[_Mount]
Expand Down Expand Up @@ -223,7 +224,8 @@ def __init__(
if image is not None and not isinstance(image, _Image):
raise InvalidError("image has to be a modal Image or AioImage object")

self._indexed_objects = {}
self._functions = {}
self._classes = {}
self._image = image
self._mounts = mounts
self._secrets = secrets
Expand Down Expand Up @@ -312,6 +314,7 @@ def _validate_blueprint_value(self, key: str, value: Any):
raise InvalidError(f"App attribute `{key}` with value {value!r} is not a valid Modal object")

def _add_object(self, tag, obj):
# TODO(erikbern): replace this with _add_function and _add_class
if self._running_app:
# If this is inside a container, then objects can be defined after app initialization.
# So we may have to initialize objects once they get bound to the app.
Expand All @@ -320,7 +323,12 @@ def _add_object(self, tag, obj):
metadata: Message = self._running_app.object_handle_metadata[object_id]
obj._hydrate(object_id, self._client, metadata)

self._indexed_objects[tag] = obj
if isinstance(obj, _Function):
self._functions[tag] = obj
elif isinstance(obj, _Cls):
self._classes[tag] = obj
else:
raise RuntimeError(f"Expected `obj` to be a _Function or _Cls (got {type(obj)}")

def __getitem__(self, tag: str):
deprecation_error((2024, 3, 25), _app_attr_error)
Expand All @@ -334,7 +342,7 @@ def __getattr__(self, tag: str):
if tag.startswith("__"):
# Hacky way to avoid certain issues, e.g. pickle will try to look this up
raise AttributeError(f"App has no member {tag}")
if tag not in self._indexed_objects:
if tag not in self._functions or tag not in self._classes:
# Primarily to make hasattr work
raise AttributeError(f"App has no member {tag}")
deprecation_error((2024, 3, 25), _app_attr_error)
Expand All @@ -360,7 +368,9 @@ def image(self, value):

def _uncreate_all_objects(self):
# TODO(erikbern): this doesn't unhydrate objects that aren't tagged
for obj in self._indexed_objects.values():
for obj in self._functions.values():
obj._unhydrate()
for obj in self._classes.values():
obj._unhydrate()

@asynccontextmanager
Expand Down Expand Up @@ -459,18 +469,17 @@ def _get_watch_mounts(self):
return [m for m in all_mounts if m.is_local()]

def _add_function(self, function: _Function, is_web_endpoint: bool):
if function.tag in self._indexed_objects:
old_function = self._indexed_objects[function.tag]
if isinstance(old_function, _Function):
if not is_notebook():
logger.warning(
f"Warning: Tag '{function.tag}' collision!"
" Overriding existing function "
f"[{old_function._info.module_name}].{old_function._info.function_name}"
f" with new function [{function._info.module_name}].{function._info.function_name}"
)
else:
logger.warning(f"Warning: tag {function.tag} exists but is overridden by function")
if function.tag in self._functions:
if not is_notebook():
old_function: _Function = self._functions[function.tag]
logger.warning(
f"Warning: Tag '{function.tag}' collision!"
" Overriding existing function "
f"[{old_function._info.module_name}].{old_function._info.function_name}"
f" with new function [{function._info.module_name}].{function._info.function_name}"
)
if function.tag in self._classes:
logger.warning(f"Warning: tag {function.tag} exists but is overridden by function")

self._add_object(function.tag, function)
if is_web_endpoint:
Expand All @@ -484,21 +493,22 @@ def _init_container(self, client: _Client, running_app: RunningApp):
_App._container_app = running_app

# Hydrate objects on app
indexed_objects = dict(**self._functions, **self._classes)
for tag, object_id in running_app.tag_to_object_id.items():
if tag in self._indexed_objects:
obj = self._indexed_objects[tag]
if tag in indexed_objects:
obj = indexed_objects[tag]
handle_metadata = running_app.object_handle_metadata[object_id]
obj._hydrate(object_id, client, handle_metadata)

@property
def registered_functions(self) -> Dict[str, _Function]:
"""All modal.Function objects registered on the app."""
return {tag: obj for tag, obj in self._indexed_objects.items() if isinstance(obj, _Function)}
return self._functions

@property
def registered_classes(self) -> Dict[str, _Function]:
"""All modal.Cls objects registered on the app."""
return {tag: obj for tag, obj in self._indexed_objects.items() if isinstance(obj, _Cls)}
return self._classes

@property
def registered_entrypoints(self) -> Dict[str, _LocalEntrypoint]:
Expand All @@ -507,7 +517,11 @@ def registered_entrypoints(self) -> Dict[str, _LocalEntrypoint]:

@property
def indexed_objects(self) -> Dict[str, _Object]:
return self._indexed_objects
deprecation_warning(
(2024, 11, 25),
"`app.indexed_objects` is deprecated! Use `app.registered_functions` or `app.registered_classes` instead.",
)
return dict(**self._functions, **self._classes)

@property
def registered_web_endpoints(self) -> List[str]:
Expand Down Expand Up @@ -1002,8 +1016,9 @@ def main():
bar.remote()
```
"""
for tag, object in other_app._indexed_objects.items():
existing_object = self._indexed_objects.get(tag)
indexed_objects = dict(**other_app._functions, **other_app._classes)
for tag, object in indexed_objects.items():
existing_object = indexed_objects.get(tag)
if existing_object and existing_object != object:
logger.warning(
f"Named app object {tag} with existing value {existing_object} is being "
Expand Down
2 changes: 1 addition & 1 deletion modal/cli/import_refs.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def _infer_function_or_help(
# entrypoint is in entrypoint registry, for now
return app.registered_entrypoints[function_name]

function = app.indexed_objects[function_name] # functions are in blueprint
function = app.registered_functions[function_name]
assert isinstance(function, Function)
return function

Expand Down
10 changes: 6 additions & 4 deletions modal/cli/launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
)


def _launch_program(name: str, filename: str, args: Dict[str, Any]) -> None:
def _launch_program(name: str, filename: str, detach: bool, args: Dict[str, Any]) -> None:
os.environ["MODAL_LAUNCH_ARGS"] = json.dumps(args)

program_path = str(Path(__file__).parent / "programs" / filename)
Expand All @@ -37,7 +37,7 @@ def _launch_program(name: str, filename: str, args: Dict[str, Any]) -> None:
func = entrypoint.info.raw_f
isasync = inspect.iscoroutinefunction(func)
with enable_output():
with run_app(app):
with run_app(app, detach=detach):
try:
if isasync:
asyncio.run(func())
Expand All @@ -57,6 +57,7 @@ def jupyter(
add_python: Optional[str] = "3.11",
mount: Optional[str] = None, # Create a `modal.Mount` from a local directory.
volume: Optional[str] = None, # Attach a persisted `modal.Volume` by name (creating if missing).
detach: bool = False, # Run the app in "detached" mode to persist after local client disconnects
):
args = {
"cpu": cpu,
Expand All @@ -68,7 +69,7 @@ def jupyter(
"mount": mount,
"volume": volume,
}
_launch_program("jupyter", "run_jupyter.py", args)
_launch_program("jupyter", "run_jupyter.py", detach, args)


@launch_cli.command(name="vscode", help="Start Visual Studio Code on Modal.")
Expand All @@ -79,6 +80,7 @@ def vscode(
timeout: int = 3600,
mount: Optional[str] = None, # Create a `modal.Mount` from a local directory.
volume: Optional[str] = None, # Attach a persisted `modal.Volume` by name (creating if missing).
detach: bool = False, # Run the app in "detached" mode to persist after local client disconnects
):
args = {
"cpu": cpu,
Expand All @@ -88,4 +90,4 @@ def vscode(
"mount": mount,
"volume": volume,
}
_launch_program("vscode", "vscode.py", args)
_launch_program("vscode", "vscode.py", detach, args)
6 changes: 3 additions & 3 deletions modal/cli/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ def _get_clean_app_description(func_ref: str) -> str:


def _get_click_command_for_function(app: App, function_tag):
function = app.indexed_objects.get(function_tag, None)
function = app.registered_functions.get(function_tag, None)
if not function or (isinstance(function, Function) and function.info.user_cls is not None):
# This is either a function_tag for a class method function (e.g MyClass.foo) or a function tag for a
# class service function (MyClass.*)
class_name, method_name = function_tag.rsplit(".", 1)
if not function:
function = app.indexed_objects.get(f"{class_name}.*")
function = app.registered_functions.get(f"{class_name}.*")
assert isinstance(function, Function)
function = typing.cast(Function, function)
if function.is_generator:
Expand All @@ -151,7 +151,7 @@ def _get_click_command_for_function(app: App, function_tag):
signature: Dict[str, ParameterMetadata]
cls: Optional[Cls] = None
if function.info.user_cls is not None:
cls = typing.cast(Cls, app.indexed_objects[class_name])
cls = typing.cast(Cls, app.registered_classes[class_name])
cls_signature = _get_signature(function.info.user_cls)
if method_name == "*":
method_names = list(cls._get_partial_functions().keys())
Expand Down
18 changes: 12 additions & 6 deletions modal/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,13 @@ def heartbeat():
)

try:
indexed_objects = dict(**app._functions, **app._classes) # TODO(erikbern): remove

# Create all members
await _create_all_objects(client, running_app, app._indexed_objects, environment_name)
await _create_all_objects(client, running_app, indexed_objects, environment_name)

# Publish the app
await _publish_app(client, running_app, app_state, app._indexed_objects)
await _publish_app(client, running_app, app_state, indexed_objects)
except asyncio.CancelledError as e:
# this typically happens on sigint/ctrl-C during setup (the KeyboardInterrupt happens in the main thread)
if output_mgr := _get_output_manager():
Expand Down Expand Up @@ -424,16 +426,18 @@ async def _serve_update(
try:
running_app: RunningApp = await _init_local_app_existing(client, existing_app_id, environment_name)

indexed_objects = dict(**app._functions, **app._classes) # TODO(erikbern): remove

# Create objects
await _create_all_objects(
client,
running_app,
app._indexed_objects,
indexed_objects,
environment_name,
)

# Publish the updated app
await _publish_app(client, running_app, api_pb2.APP_STATE_UNSPECIFIED, app._indexed_objects)
await _publish_app(client, running_app, api_pb2.APP_STATE_UNSPECIFIED, indexed_objects)

# Communicate to the parent process
is_ready.set()
Expand Down Expand Up @@ -521,17 +525,19 @@ def heartbeat():

tc.infinite_loop(heartbeat, sleep=HEARTBEAT_INTERVAL)

indexed_objects = dict(**app._functions, **app._classes) # TODO(erikbern): remove

try:
# Create all members
await _create_all_objects(
client,
running_app,
app._indexed_objects,
indexed_objects,
environment_name=environment_name,
)

app_url, warnings = await _publish_app(
client, running_app, api_pb2.APP_STATE_DEPLOYED, app._indexed_objects, name, tag
client, running_app, api_pb2.APP_STATE_DEPLOYED, indexed_objects, name, tag
)
except Exception as e:
# Note that AppClientDisconnect only stops the app if it's still initializing, and is a no-op otherwise.
Expand Down
2 changes: 1 addition & 1 deletion modal_version/_version_generated.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright Modal Labs 2024

# Note: Reset this value to -1 whenever you make a minor `0.X` release of the client.
build_number = -1 # git: 10a2cbc
build_number = -1 # git: 6dcd0c7
4 changes: 2 additions & 2 deletions test/function_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ def test_default_cloud_provider(client, servicer, monkeypatch):
monkeypatch.setenv("MODAL_DEFAULT_CLOUD", "oci")
app.function()(dummy)
with app.run(client=client):
object_id: str = app.indexed_objects["dummy"].object_id
object_id: str = app.registered_functions["dummy"].object_id
f = servicer.app_functions[object_id]

assert f.cloud_provider == api_pb2.CLOUD_PROVIDER_OCI
Expand Down Expand Up @@ -828,7 +828,7 @@ def test_deps_explicit(client, servicer):
app.function(image=image, network_file_systems={"/nfs_1": nfs_1, "/nfs_2": nfs_2})(dummy)

with app.run(client=client):
object_id: str = app.indexed_objects["dummy"].object_id
object_id: str = app.registered_functions["dummy"].object_id
f = servicer.app_functions[object_id]

dep_object_ids = set(d.object_id for d in f.object_dependencies)
Expand Down
3 changes: 1 addition & 2 deletions test/live_reload_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ async def fake_watch():

with serve_app(app, app_ref, _watcher=fake_watch()):
watcher_done.wait() # wait until watcher loop is done
foo = app.indexed_objects["foo"]
assert isinstance(foo, Function)
foo: Function = app.registered_functions["foo"]
assert foo.web_url.startswith("http://")

# TODO ideally we would assert the specific expected number here, but this test
Expand Down

0 comments on commit 19e63fc

Please sign in to comment.