From 2ddd196e19ac862b2ce2f4651ae716eadc0968d7 Mon Sep 17 00:00:00 2001 From: Rick Izzo Date: Wed, 9 Nov 2022 22:38:42 -0500 Subject: [PATCH 1/5] fixed condition in which app command comments would not execute before the flow is dispatched --- src/lightning_app/cli/lightning_cli.py | 7 ++++- src/lightning_app/runners/multiprocess.py | 6 ---- src/lightning_app/runners/singleprocess.py | 7 ----- .../test_installation_commands_app.py | 28 +++++++++++++++++-- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/lightning_app/cli/lightning_cli.py b/src/lightning_app/cli/lightning_cli.py index 471e1cdc15dca..dcacd47c2e963 100644 --- a/src/lightning_app/cli/lightning_cli.py +++ b/src/lightning_app/cli/lightning_cli.py @@ -29,9 +29,10 @@ from lightning_app.cli.lightning_cli_delete import delete from lightning_app.cli.lightning_cli_list import get_list from lightning_app.cli.lightning_cli_remove import cli_remove -from lightning_app.core.constants import DEBUG, get_lightning_cloud_url +from lightning_app.core.constants import DEBUG, ENABLE_APP_COMMENT_COMMAND_EXECUTION, get_lightning_cloud_url from lightning_app.runners.runtime import dispatch from lightning_app.runners.runtime_type import RuntimeType +from lightning_app.utilities.app_commands import run_app_commands from lightning_app.utilities.app_helpers import Logger from lightning_app.utilities.cli_helpers import ( _arrow_time_callback, @@ -260,6 +261,9 @@ def _run_app( "Secrets can only be used for apps running in cloud. " "Using the option --secret in local execution is not supported." ) + if ENABLE_APP_COMMENT_COMMAND_EXECUTION or run_app_comment_commands: + if file is not None: + run_app_commands(str(file)) env_vars = _format_input_env_variables(env) os.environ.update(env_vars) @@ -271,6 +275,7 @@ def on_before_run(*args: Any, **kwargs: Any) -> None: click.launch(get_app_url(runtime_type, *args, **kwargs)) click.echo("Your Lightning App is starting. This won't take long.") + click.echo(f"{run_app_comment_commands=}") # TODO: Fixme when Grid utilities are available. # And refactor test_lightning_run_app_cloud diff --git a/src/lightning_app/runners/multiprocess.py b/src/lightning_app/runners/multiprocess.py index 7e8719e9fe26c..1bc8c7b5cf178 100644 --- a/src/lightning_app/runners/multiprocess.py +++ b/src/lightning_app/runners/multiprocess.py @@ -5,11 +5,9 @@ from lightning_app.api.http_methods import _add_tags_to_api, _validate_api from lightning_app.core.api import start_server -from lightning_app.core.constants import ENABLE_APP_COMMENT_COMMAND_EXECUTION from lightning_app.runners.backends import Backend from lightning_app.runners.runtime import Runtime from lightning_app.storage.orchestrator import StorageOrchestrator -from lightning_app.utilities.app_commands import run_app_commands from lightning_app.utilities.app_helpers import is_overridden from lightning_app.utilities.commands.base import _commands_to_api, _prepare_commands from lightning_app.utilities.component import _set_flow_context, _set_frontend_context @@ -30,10 +28,6 @@ class MultiProcessRuntime(Runtime): def dispatch(self, *args: Any, on_before_run: Optional[Callable] = None, **kwargs: Any): """Method to dispatch and run the LightningApp.""" - if ENABLE_APP_COMMENT_COMMAND_EXECUTION or self.run_app_comment_commands: - if self.entrypoint_file is not None: - run_app_commands(str(self.entrypoint_file)) - try: _set_flow_context() self.app.backend = self.backend diff --git a/src/lightning_app/runners/singleprocess.py b/src/lightning_app/runners/singleprocess.py index 435d30c5b9b06..b12b86e8625aa 100644 --- a/src/lightning_app/runners/singleprocess.py +++ b/src/lightning_app/runners/singleprocess.py @@ -2,10 +2,8 @@ from typing import Any, Callable, Optional from lightning_app.core.api import start_server -from lightning_app.core.constants import ENABLE_APP_COMMENT_COMMAND_EXECUTION from lightning_app.core.queues import QueuingSystem from lightning_app.runners.runtime import Runtime -from lightning_app.utilities.app_commands import run_app_commands from lightning_app.utilities.load_app import extract_metadata_from_app @@ -17,11 +15,6 @@ def __post_init__(self): def dispatch(self, *args, on_before_run: Optional[Callable] = None, **kwargs: Any): """Method to dispatch and run the LightningApp.""" - - if ENABLE_APP_COMMENT_COMMAND_EXECUTION or self.run_app_comment_commands: - if self.entrypoint_file is not None: - run_app_commands(str(self.entrypoint_file)) - queue = QueuingSystem.SINGLEPROCESS self.app.delta_queue = queue.get_delta_queue() diff --git a/tests/tests_app_examples/test_installation_commands_app.py b/tests/tests_app_examples/test_installation_commands_app.py index 84cec7d505efa..d7267b746ad07 100644 --- a/tests/tests_app_examples/test_installation_commands_app.py +++ b/tests/tests_app_examples/test_installation_commands_app.py @@ -7,18 +7,40 @@ from lightning_app.testing.testing import run_app_in_cloud -@pytest.mark.skip(reason="temporarily disabled until backend release") @pytest.mark.cloud def test_installation_commands_app_example_cloud() -> None: + # This is expected to fail since it is missing the "setup" flag with run_app_in_cloud( os.path.join(_PROJECT_ROOT, "examples/app_installation_commands"), app_name="app.py", - extra_args=["--setup"], debug=True, ) as (_, _, fetch_logs, _): has_logs = False while not has_logs: + for log in fetch_logs(["flow"]): + if "ModuleNotFoundError: No module named 'lmdb'" in log: + has_logs = True + time.sleep(1) + + # This is expected to pass, since the "setup" flag is passed + with run_app_in_cloud( + os.path.join(_PROJECT_ROOT, "examples/app_installation_commands"), + app_name="app.py", + extra_args=["--setup"], + debug=True, + ) as (_, _, fetch_logs, _): + # check installation in flow + has_flow_logs = False + while not has_flow_logs: + for log in fetch_logs(["flow"]): + if "lmdb successfully installed" in log: + has_flow_logs = True + time.sleep(1) + + # check installation in work + has_work_logs = False + while not has_work_logs: for log in fetch_logs(["work"]): if "lmdb successfully installed" in log: - has_logs = True + has_work_logs = True time.sleep(1) From 9eccf9a52105974c767f3f176a9785278928ad40 Mon Sep 17 00:00:00 2001 From: Rick Izzo Date: Wed, 9 Nov 2022 22:59:28 -0500 Subject: [PATCH 2/5] remove debug print statment --- src/lightning_app/cli/lightning_cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lightning_app/cli/lightning_cli.py b/src/lightning_app/cli/lightning_cli.py index dcacd47c2e963..5a41f4b511ac0 100644 --- a/src/lightning_app/cli/lightning_cli.py +++ b/src/lightning_app/cli/lightning_cli.py @@ -275,7 +275,6 @@ def on_before_run(*args: Any, **kwargs: Any) -> None: click.launch(get_app_url(runtime_type, *args, **kwargs)) click.echo("Your Lightning App is starting. This won't take long.") - click.echo(f"{run_app_comment_commands=}") # TODO: Fixme when Grid utilities are available. # And refactor test_lightning_run_app_cloud From 592298d746371d249dd506cb2199897f31ef5db7 Mon Sep 17 00:00:00 2001 From: Rick Izzo Date: Wed, 9 Nov 2022 23:04:11 -0500 Subject: [PATCH 3/5] updated example code --- examples/app_installation_commands/app.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/app_installation_commands/app.py b/examples/app_installation_commands/app.py index 90effc631daa5..9eb1c2944ee2e 100644 --- a/examples/app_installation_commands/app.py +++ b/examples/app_installation_commands/app.py @@ -9,8 +9,12 @@ class YourComponent(L.LightningWork): def run(self): - print(lmdb.__version__) + print(lmdb.version()) print("lmdb successfully installed") + print("accessing a module in a Work or Flow body works!") + + +print(f"accessing an object in main code body works!: version={lmdb.version()}") # run on a cloud machine From 311155bcba1fd96a52eed0db6dcb3f875adb0566 Mon Sep 17 00:00:00 2001 From: Rick Izzo Date: Wed, 9 Nov 2022 23:13:39 -0500 Subject: [PATCH 4/5] fix test --- .../test_installation_commands_app.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/tests_app_examples/test_installation_commands_app.py b/tests/tests_app_examples/test_installation_commands_app.py index d7267b746ad07..07a52b934f16d 100644 --- a/tests/tests_app_examples/test_installation_commands_app.py +++ b/tests/tests_app_examples/test_installation_commands_app.py @@ -17,7 +17,7 @@ def test_installation_commands_app_example_cloud() -> None: ) as (_, _, fetch_logs, _): has_logs = False while not has_logs: - for log in fetch_logs(["flow"]): + for log in fetch_logs(["work"]): if "ModuleNotFoundError: No module named 'lmdb'" in log: has_logs = True time.sleep(1) @@ -29,18 +29,9 @@ def test_installation_commands_app_example_cloud() -> None: extra_args=["--setup"], debug=True, ) as (_, _, fetch_logs, _): - # check installation in flow has_flow_logs = False while not has_flow_logs: - for log in fetch_logs(["flow"]): - if "lmdb successfully installed" in log: - has_flow_logs = True - time.sleep(1) - - # check installation in work - has_work_logs = False - while not has_work_logs: for log in fetch_logs(["work"]): if "lmdb successfully installed" in log: - has_work_logs = True + has_flow_logs = True time.sleep(1) From 71d2a588759f0f077b9442c1744f0e5c4a3a39ec Mon Sep 17 00:00:00 2001 From: Rick Izzo Date: Thu, 10 Nov 2022 00:16:53 -0500 Subject: [PATCH 5/5] updates to test --- .../test_installation_commands_app.py | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/tests/tests_app_examples/test_installation_commands_app.py b/tests/tests_app_examples/test_installation_commands_app.py index 07a52b934f16d..41ba18c06bc73 100644 --- a/tests/tests_app_examples/test_installation_commands_app.py +++ b/tests/tests_app_examples/test_installation_commands_app.py @@ -1,5 +1,4 @@ import os -from datetime import time import pytest from tests_app import _PROJECT_ROOT @@ -9,19 +8,6 @@ @pytest.mark.cloud def test_installation_commands_app_example_cloud() -> None: - # This is expected to fail since it is missing the "setup" flag - with run_app_in_cloud( - os.path.join(_PROJECT_ROOT, "examples/app_installation_commands"), - app_name="app.py", - debug=True, - ) as (_, _, fetch_logs, _): - has_logs = False - while not has_logs: - for log in fetch_logs(["work"]): - if "ModuleNotFoundError: No module named 'lmdb'" in log: - has_logs = True - time.sleep(1) - # This is expected to pass, since the "setup" flag is passed with run_app_in_cloud( os.path.join(_PROJECT_ROOT, "examples/app_installation_commands"), @@ -29,9 +15,8 @@ def test_installation_commands_app_example_cloud() -> None: extra_args=["--setup"], debug=True, ) as (_, _, fetch_logs, _): - has_flow_logs = False - while not has_flow_logs: + has_logs = False + while not has_logs: for log in fetch_logs(["work"]): if "lmdb successfully installed" in log: - has_flow_logs = True - time.sleep(1) + has_logs = True