Skip to content

Commit

Permalink
Stub deprecation warning (#1733)
Browse files Browse the repository at this point in the history
* enable deprecation warning

* Add test for Stub() warnings

* stub_test -> app_test

* run stub warnings

* remove stub from markdown fixture

* test/stub_composition_test.py -> test/app_composition_test.py

* Rebase fix

* Enable deprecation warning for app symbol at module level

* Fix test (trigger 1 deprecation warning not 2)

* Make it pending, set tentative date to 2024-04-26

* push date off to 2024-04-29

* Add deprecation warnings for run_stub, deploy_stub, and serve_stub

* Push it off to May 1
  • Loading branch information
erikbern authored May 1, 2024
1 parent 5e99e37 commit 247c739
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 31 deletions.
1 change: 0 additions & 1 deletion conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ def pytest_markdown_docs_globals():
return {
"modal": modal,
"app": modal.App(),
"stub": modal.Stub(),
"math": math,
"__name__": "runtest",
"web_endpoint": modal.web_endpoint,
Expand Down
14 changes: 7 additions & 7 deletions modal/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ def function(

if interactive:
deprecation_error(
(2024, 2, 29), "interactive=True has been deprecated. Set MODAL_INTERACTIVE_FUNCTIONS=1 instead."
(2024, 5, 1), "interactive=True has been deprecated. Set MODAL_INTERACTIVE_FUNCTIONS=1 instead."
)

if image is None:
Expand Down Expand Up @@ -827,12 +827,12 @@ class _Stub(_App):
"""

def __new__(cls, *args, **kwargs):
# TODO(erikbern): enable this warning soon!
# deprecation_warning(
# (2024, 4, 19),
# "The use of \"Stub\" has been deprecated in favor of \"App\"."
# " This is a pure name change with no other implications."
# )
deprecation_warning(
(2024, 4, 29),
"The use of \"Stub\" has been deprecated in favor of \"App\"."
" This is a pure name change with no other implications.",
pending=True
)
return _App(*args, **kwargs)


Expand Down
12 changes: 6 additions & 6 deletions modal/cli/import_refs.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@ def get_by_object_path_try_possible_app_names(obj: Any, obj_path: Optional[str])
)
return stub
elif isinstance(stub, App):
# TODO(erikbern): enable this deprecation warning shortly
# deprecation_warning(
# (2024, 4, 20),
# "The symbol `app` is not present but `stub` is. This will not work in future"
# " Modal versions. Suggestion: change the name of `stub` to `app`."
# )
deprecation_warning(
(2024, 5, 1),
"The symbol `app` is not present but `stub` is. This will not work in future"
" Modal versions. Suggestion: change the name of `stub` to `app`.",
pending=True
)
return stub
else:
return None
Expand Down
26 changes: 19 additions & 7 deletions modal/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@
from ._utils.grpc_utils import retry_transient_errors
from .client import HEARTBEAT_INTERVAL, HEARTBEAT_TIMEOUT, _Client
from .config import config, logger
from .exception import ExecutionError, InteractiveTimeoutError, InvalidError, _CliUserExecutionError
from .exception import (
ExecutionError,
InteractiveTimeoutError,
InvalidError,
_CliUserExecutionError,
deprecation_warning,
)
from .execution_context import is_local
from .object import _Object
from .running_app import RunningApp
Expand Down Expand Up @@ -509,13 +515,19 @@ async def _interactive_shell(_app: _App, cmd: List[str], environment_name: str =
await connect_to_sandbox(sb)


def _run_stub(*args, **kwargs):
deprecation_warning((2024, 5, 1), "`run_stub` is deprecated. Please use `run_app` instead.", pending=True)
return _run_app(*args, **kwargs)


def _deploy_stub(*args, **kwargs):
deprecation_warning((2024, 5, 1), "`deploy_stub` is deprecated. Please use `deploy_app` instead.", pending=True)
return _deploy_app(*args, **kwargs)


run_app = synchronize_api(_run_app)
serve_update = synchronize_api(_serve_update)
deploy_app = synchronize_api(_deploy_app)
interactive_shell = synchronize_api(_interactive_shell)

# Soon-to-be-deprecated ones, add warning soon
_run_stub = _run_app
run_stub = run_app
_deploy_stub = _deploy_app
deploy_stub = deploy_app
run_stub = synchronize_api(_run_stub)
deploy_stub = synchronize_api(_deploy_stub)
11 changes: 7 additions & 4 deletions modal/serving.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from .cli.import_refs import import_app
from .client import _Client
from .config import config
from .exception import deprecation_warning
from .runner import _disconnect, _run_app, serve_update

if TYPE_CHECKING:
Expand Down Expand Up @@ -127,8 +128,10 @@ async def _serve_app(
yield app


serve_app = synchronize_api(_serve_app)
def _serve_stub(*args, **kwargs):
deprecation_warning((2024, 5, 1), "`serve_stub` is deprecated. Please use `serve_app` instead.", pending=True)
return _run_app(*args, **kwargs)


# Soon-to-be-deprecated ones, add warning soon
_serve_stub = _serve_app
serve_stub = serve_app
serve_app = synchronize_api(_serve_app)
serve_stub = synchronize_api(_serve_stub)
File renamed without changes.
16 changes: 14 additions & 2 deletions test/stub_test.py → test/app_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
from google.protobuf.empty_pb2 import Empty
from grpclib import GRPCError, Status

from modal import App, Dict, Image, Mount, Queue, Secret, Volume, web_endpoint
from modal import App, Dict, Image, Mount, Queue, Secret, Stub, Volume, web_endpoint
from modal.app import list_apps # type: ignore
from modal.config import config
from modal.exception import DeprecationError, ExecutionError, InvalidError, NotFoundError
from modal.partial_function import _parse_custom_domains
from modal.runner import deploy_app
from modal.runner import deploy_app, deploy_stub
from modal_proto import api_pb2

from .supports import module_1, module_2
Expand Down Expand Up @@ -359,3 +359,15 @@ def test_function_named_app():
@app.function(serialized=True)
def app():
...


def test_stub():
with pytest.warns(match="App"):
Stub()


def test_deploy_stub(servicer, client):
app = App("xyz")
deploy_app(app, client=client)
with pytest.warns(match="deploy_app"):
deploy_stub(app, client=client)
11 changes: 10 additions & 1 deletion test/cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,16 @@ def test_run(servicer, set_env_client, test_dir):

def test_run_stub(servicer, set_env_client, test_dir):
app_file = test_dir / "supports" / "app_run_tests" / "app_was_once_stub.py"
_run(["run", app_file.as_posix()])
with pytest.warns(match="App"):
_run(["run", app_file.as_posix()])
with pytest.warns(match="App"):
_run(["run", app_file.as_posix() + "::foo"])


def test_run_stub_2(servicer, set_env_client, test_dir):
app_file = test_dir / "supports" / "app_run_tests" / "app_was_once_stub_2.py"
with pytest.warns(match="`app`"):
_run(["run", app_file.as_posix()])
_run(["run", app_file.as_posix() + "::stub"])
_run(["run", app_file.as_posix() + "::foo"])

Expand Down
2 changes: 1 addition & 1 deletion test/supports/app_run_tests/app_and_stub.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# rather than complaining about the type of `app`
app = 123

stub = modal.Stub()
stub = modal.App()


@stub.function()
Expand Down
6 changes: 4 additions & 2 deletions test/supports/app_run_tests/app_was_once_stub.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# Copyright Modal Labs 2024
import modal

stub = modal.Stub()
# Trigger the deprecation warning for the class itself,
# but not the symbol name (see app_was_once_stub_2 for the opposite)
app = modal.Stub()


@stub.function()
@app.function()
def foo():
print("foo")
11 changes: 11 additions & 0 deletions test/supports/app_run_tests/app_was_once_stub_2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Copyright Modal Labs 2024
import modal

# Don't trigger the deprecation warning for the class itself,
# but there should be a separate deprecation warning because of the symbol name
stub = modal.App()


@stub.function()
def foo():
print("foo")

0 comments on commit 247c739

Please sign in to comment.