Move to next phase of show_progress deprecation (#2548)

Move to next phase of show_progress deprecation

modal/app.py

@@ -42,7 +42,6 @@
 from .mount import _Mount
 from .network_file_system import _NetworkFileSystem
 from .object import _get_environment_name, _Object
-from .output import _get_output_manager, enable_output
 from .partial_function import (
     PartialFunction,
     _find_partial_methods_for_user_cls,
@@ -141,21 +140,6 @@ def f(x, y):
 ```
 """
 
-_enable_output_warning = """\
-Note that output will soon not be be printed with `app.run`.
-
-If you want to print output, use `modal.enable_output()`:
-
-```python
-with modal.enable_output():
-    with app.run():
-        ...
-```
-
-If you don't want output, and you want to to suppress this warning,
-use `app.run(..., show_progress=False)`.
-"""
-
 
 class _App:
     """A Modal App is a group of functions and classes that are deployed together.
@@ -446,32 +430,18 @@ async def run(
 
         # See Github discussion here: https://github.com/modal-labs/modal-client/pull/2030#issuecomment-2237266186
 
-        auto_enable_output = False
-
-        if "MODAL_DISABLE_APP_RUN_OUTPUT_WARNING" not in os.environ:
-            if show_progress is None:
-                if _get_output_manager() is None:
-                    deprecation_warning((2024, 7, 18), _enable_output_warning)
-                    auto_enable_output = True
-            elif show_progress is True:
-                if _get_output_manager() is None:
-                    deprecation_warning((2024, 7, 18), _enable_output_warning)
-                    auto_enable_output = True
-                else:
-                    deprecation_warning((2024, 7, 18), "`show_progress=True` is deprecated and no longer needed.")
-            elif show_progress is False:
-                if _get_output_manager() is not None:
-                    deprecation_warning(
-                        (2024, 7, 18), "`show_progress=False` will have no effect since output is enabled."
-                    )
+        if show_progress is True:
+            deprecation_error(
+                (2024, 11, 20),
+                "`show_progress=True` is no longer supported. Use `with modal.enable_output():` instead.",
+            )
+        elif show_progress is False:
+            # TODO(erikbern): remove this env check very shortly
+            if "MODAL_DISABLE_APP_RUN_OUTPUT_WARNING" not in os.environ:
+                deprecation_warning((2024, 11, 20), "`show_progress=False` is deprecated (and has no effect)")
 
-        if auto_enable_output:
-            with enable_output():
-                async with _run_app(self, client=client, detach=detach, interactive=interactive):
-                    yield self
-        else:
-            async with _run_app(self, client=client, detach=detach, interactive=interactive):
-                yield self
+        async with _run_app(self, client=client, detach=detach, interactive=interactive):
+            yield self
 
     def _get_default_image(self):
         if self._image:

test/app_test.py

@@ -10,7 +10,6 @@
 from modal import App, Dict, Image, Mount, Secret, Stub, Volume, enable_output, web_endpoint
 from modal._utils.async_utils import synchronizer
 from modal.exception import DeprecationError, ExecutionError, InvalidError, NotFoundError
-from modal.output import _get_output_manager
 from modal.partial_function import _parse_custom_domains
 from modal.runner import deploy_app, deploy_stub, run_app
 from modal_proto import api_pb2
@@ -405,42 +404,22 @@ async def app_logs_pty(servicer, stream):
 
 
 def test_show_progress_deprecations(client, monkeypatch):
-    # Unset env used to disable warning
-    monkeypatch.delenv("MODAL_DISABLE_APP_RUN_OUTPUT_WARNING")
-
     app = App()
 
-    # If show_progress is not provided, and output is not enabled, warn
-    with pytest.warns(DeprecationError, match="enable_output"):
-        with app.run(client=client):
-            assert _get_output_manager() is not None  # Should be auto-enabled
-
-    # If show_progress is not provided, and output is enabled, no warning
-    with enable_output():
-        with app.run(client=client):
-            pass
-
-    # If show_progress is set to True, and output is not enabled, warn
-    with pytest.warns(DeprecationError, match="enable_output"):
+    # If show_progress is set to True, raise that this is deprecated
+    with pytest.raises(DeprecationError, match="enable_output"):
         with app.run(client=client, show_progress=True):
-            assert _get_output_manager() is not None  # Should be auto-enabled
+            pass
 
-    # If show_progress is set to True, and output is enabled, warn the flag is superfluous
-    with pytest.warns(DeprecationError, match="`show_progress=True` is deprecated"):
-        with enable_output():
-            with app.run(client=client, show_progress=True):
-                pass
+    # If show_progress is set to False, warn that this has no effect
+    with pytest.warns(DeprecationError, match="no effect"):
+        with app.run(client=client, show_progress=False):
+            pass
 
-    # If show_progress is set to False, and output is not enabled, no warning
-    # This mode is currently used to suppress deprecation warnings, but will in itself be deprecated later.
+    # This is used by integration tests for a short period
+    monkeypatch.setenv("MODAL_DISABLE_APP_RUN_OUTPUT_WARNING", "1")
     with app.run(client=client, show_progress=False):
-        assert _get_output_manager() is None
-
-    # If show_progress is set to False, and output is enabled, warn that it has no effect
-    with pytest.warns(DeprecationError, match="no effect"):
-        with enable_output():
-            with app.run(client=client, show_progress=False):
-                pass
+        pass
 
 
 @pytest.mark.asyncio

test/conftest.py

@@ -61,11 +61,6 @@ def set_env(monkeypatch):
     monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
 
 
-@pytest.fixture(scope="function", autouse=True)
-def disable_app_run_warning(monkeypatch):
-    monkeypatch.setenv("MODAL_DISABLE_APP_RUN_OUTPUT_WARNING", "1")
-
-
 @pytest.fixture(scope="function", autouse=True)
 def ignore_local_config():
     # When running tests locally, we don't want to pick up the local .modal.toml file