From 804f7fb9267b7b0c1e2eb2d95ed033f251ae2397 Mon Sep 17 00:00:00 2001 From: Ethan Harris Date: Fri, 11 Nov 2022 18:24:17 +0000 Subject: [PATCH 1/5] Change app root / config path to be the `app.py` parent directory --- src/lightning_app/runners/cloud.py | 25 ++++++++++++++++--- .../utilities/packaging/app_config.py | 18 ++++--------- .../utilities/packaging/test_app_config.py | 22 ++++------------ 3 files changed, 31 insertions(+), 34 deletions(-) diff --git a/src/lightning_app/runners/cloud.py b/src/lightning_app/runners/cloud.py index 81e9e10ebb14a..662b78c6d64d2 100644 --- a/src/lightning_app/runners/cloud.py +++ b/src/lightning_app/runners/cloud.py @@ -63,7 +63,7 @@ from lightning_app.utilities.cloud import _get_project from lightning_app.utilities.dependency_caching import get_hash from lightning_app.utilities.load_app import _prettifiy_exception, load_app_from_file -from lightning_app.utilities.packaging.app_config import AppConfig, find_config_file +from lightning_app.utilities.packaging.app_config import _get_config_file, AppConfig from lightning_app.utilities.packaging.lightning_utils import _prepare_lightning_wheels_and_requirements from lightning_app.utilities.secrets import _names_to_ids @@ -95,9 +95,9 @@ def dispatch( # TODO: verify lightning version # _verify_lightning_version() - config_file = find_config_file(self.entrypoint_file) - app_config = AppConfig.load_from_file(config_file) if config_file else AppConfig() - root = config_file.parent if config_file else Path(self.entrypoint_file).absolute().parent + config_file = _get_config_file(self.entrypoint_file) + app_config = AppConfig.load_from_file(config_file) if config_file.exists() else AppConfig() + root = Path(self.entrypoint_file).absolute().parent cleanup_handle = _prepare_lightning_wheels_and_requirements(root) repo = LocalSourceCodeDir(path=root) self._check_uploaded_folder(root, repo) @@ -141,6 +141,7 @@ def dispatch( v1_env_vars.append(V1EnvVar(name="ENABLE_PUSHING_STATE_ENDPOINT", value="0")) works: List[V1Work] = [] + dict_works = [] for flow in self.app.flows: for work in flow.works(recurse=False): if not work._start_with_flow: @@ -214,6 +215,18 @@ def dispatch( ) works.append(V1Work(name=work.name, spec=work_spec)) + dict_works.append( + { + "name": work.name, + "spec": { + "build_spec": build_spec.to_dict(), + "drives": [spec.to_dict() for spec in drive_specs], + "user_requested_compute_config": user_compute_config.to_dict(), + "network_config": [{"name": random_name, "port": work.port}], + }, + } + ) + # We need to collect a spec for each flow that contains a frontend so that the backend knows # for which flows it needs to start servers by invoking the cli (see the serve_frontend() method below) frontend_specs: List[V1Flowserver] = [] @@ -341,6 +354,10 @@ def dispatch( if app_config.cluster_id is not None: self._ensure_cluster_project_binding(project.project_id, app_config.cluster_id) + import json + + print(json.dumps(dict_works)) + release_body = Body8( app_entrypoint_file=app_spec.app_entrypoint_file, enable_app_server=app_spec.enable_app_server, diff --git a/src/lightning_app/utilities/packaging/app_config.py b/src/lightning_app/utilities/packaging/app_config.py index 59d05debc088c..9015e31363482 100644 --- a/src/lightning_app/utilities/packaging/app_config.py +++ b/src/lightning_app/utilities/packaging/app_config.py @@ -28,7 +28,7 @@ def save_to_file(self, path: Union[str, pathlib.Path]) -> None: def save_to_dir(self, directory: Union[str, pathlib.Path]) -> None: """Save the configuration to a file '.lightning' to the given folder in YAML format.""" - self.save_to_file(pathlib.Path(directory, _APP_CONFIG_FILENAME)) + self.save_to_file(_get_config_file(directory)) @classmethod def load_from_file(cls, path: Union[str, pathlib.Path]) -> "AppConfig": @@ -47,22 +47,14 @@ def load_from_dir(cls, directory: Union[str, pathlib.Path]) -> "AppConfig": return cls.load_from_file(pathlib.Path(directory, _APP_CONFIG_FILENAME)) -def find_config_file(source_path: pathlib.Path = pathlib.Path.cwd()) -> Optional[pathlib.Path]: - """Search for the Lightning app config file '.lightning' at the given source path. - - Relative to the given path, it will search for the '.lightning' config file by going up the directory structure - until found. Returns ``None`` if no config file is found in any of the parent directories. +def _get_config_file(source_path: pathlib.Path) -> Optional[pathlib.Path]: + """Get the Lightning app config file '.lightning' at the given source path. Args: - source_path: A path to a folder or a file. The search for the config file will start relative to this path. + source_path: A path to a folder or a file. """ source_path = pathlib.Path(source_path).absolute() if source_path.is_file(): source_path = source_path.parent - candidate = pathlib.Path(source_path / _APP_CONFIG_FILENAME) - if candidate.is_file(): - return candidate - - if source_path.parents: - return find_config_file(source_path.parent) + return pathlib.Path(source_path / _APP_CONFIG_FILENAME) diff --git a/tests/tests_app/utilities/packaging/test_app_config.py b/tests/tests_app/utilities/packaging/test_app_config.py index 2666f0a769ace..60da494a47fb8 100644 --- a/tests/tests_app/utilities/packaging/test_app_config.py +++ b/tests/tests_app/utilities/packaging/test_app_config.py @@ -1,6 +1,6 @@ import pathlib -from lightning_app.utilities.packaging.app_config import AppConfig, find_config_file +from lightning_app.utilities.packaging.app_config import _get_config_file, AppConfig def _make_empty_config_file(folder): @@ -10,24 +10,12 @@ def _make_empty_config_file(folder): return file -def test_find_config_file(tmpdir, monkeypatch): - monkeypatch.chdir(pathlib.Path("/")) - assert find_config_file() is None - - monkeypatch.chdir(pathlib.Path.home()) - assert find_config_file() is None - +def test_get_config_file(tmpdir): _ = _make_empty_config_file(tmpdir) - config_file1 = _make_empty_config_file(tmpdir / "a" / "b") - - assert find_config_file(tmpdir) == pathlib.Path(tmpdir, ".lightning") - assert find_config_file(config_file1) == pathlib.Path(tmpdir, "a", "b", ".lightning") - assert find_config_file(pathlib.Path(tmpdir, "a")) == pathlib.Path(tmpdir, ".lightning") + config_file1 = _make_empty_config_file(tmpdir) - # the config must be a file, a folder of the same name gets ignored - fake_config_folder = pathlib.Path(tmpdir, "fake", ".lightning") - fake_config_folder.mkdir(parents=True) - assert find_config_file(tmpdir) == pathlib.Path(tmpdir, ".lightning") + assert _get_config_file(tmpdir) == pathlib.Path(tmpdir, ".lightning") + assert _get_config_file(config_file1) == pathlib.Path(tmpdir, ".lightning") def test_app_config_save_load(tmpdir): From ca08a147b99709be20fdbf7907266c7f15ebe0f1 Mon Sep 17 00:00:00 2001 From: Ethan Harris Date: Fri, 11 Nov 2022 18:30:34 +0000 Subject: [PATCH 2/5] Update CHANGELOG.md --- src/lightning_app/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/lightning_app/CHANGELOG.md b/src/lightning_app/CHANGELOG.md index 436d6d0d651e1..9c8382a2f57f0 100644 --- a/src/lightning_app/CHANGELOG.md +++ b/src/lightning_app/CHANGELOG.md @@ -33,6 +33,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - The `params` argument in `TracerPythonScript.run` no longer prepends `--` automatically to parameters ([#15518](https://github.com/Lightning-AI/lightning/pull/15518)) +- Changed the root directory of the app (which gets uploaded) to be the folder containing the app file, rather than any parent folder containing a `.lightning` file ([#15654](https://github.com/Lightning-AI/lightning/pull/15654)) + ### Deprecated From 0fe2e0d2d71073bb0d0826a720a2946cc2603e50 Mon Sep 17 00:00:00 2001 From: Ethan Harris Date: Fri, 11 Nov 2022 18:35:02 +0000 Subject: [PATCH 3/5] mypy --- src/lightning_app/utilities/packaging/app_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning_app/utilities/packaging/app_config.py b/src/lightning_app/utilities/packaging/app_config.py index 9015e31363482..f4791eebabdaf 100644 --- a/src/lightning_app/utilities/packaging/app_config.py +++ b/src/lightning_app/utilities/packaging/app_config.py @@ -47,7 +47,7 @@ def load_from_dir(cls, directory: Union[str, pathlib.Path]) -> "AppConfig": return cls.load_from_file(pathlib.Path(directory, _APP_CONFIG_FILENAME)) -def _get_config_file(source_path: pathlib.Path) -> Optional[pathlib.Path]: +def _get_config_file(source_path: Union[str, pathlib.Path]) -> Optional[pathlib.Path]: """Get the Lightning app config file '.lightning' at the given source path. Args: From bc81cf2434e232db34105de9051e12e419186e94 Mon Sep 17 00:00:00 2001 From: Ethan Harris Date: Fri, 11 Nov 2022 18:44:34 +0000 Subject: [PATCH 4/5] Fix --- src/lightning_app/runners/cloud.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/lightning_app/runners/cloud.py b/src/lightning_app/runners/cloud.py index 662b78c6d64d2..3963db47683e7 100644 --- a/src/lightning_app/runners/cloud.py +++ b/src/lightning_app/runners/cloud.py @@ -141,7 +141,6 @@ def dispatch( v1_env_vars.append(V1EnvVar(name="ENABLE_PUSHING_STATE_ENDPOINT", value="0")) works: List[V1Work] = [] - dict_works = [] for flow in self.app.flows: for work in flow.works(recurse=False): if not work._start_with_flow: @@ -215,18 +214,6 @@ def dispatch( ) works.append(V1Work(name=work.name, spec=work_spec)) - dict_works.append( - { - "name": work.name, - "spec": { - "build_spec": build_spec.to_dict(), - "drives": [spec.to_dict() for spec in drive_specs], - "user_requested_compute_config": user_compute_config.to_dict(), - "network_config": [{"name": random_name, "port": work.port}], - }, - } - ) - # We need to collect a spec for each flow that contains a frontend so that the backend knows # for which flows it needs to start servers by invoking the cli (see the serve_frontend() method below) frontend_specs: List[V1Flowserver] = [] @@ -354,10 +341,6 @@ def dispatch( if app_config.cluster_id is not None: self._ensure_cluster_project_binding(project.project_id, app_config.cluster_id) - import json - - print(json.dumps(dict_works)) - release_body = Body8( app_entrypoint_file=app_spec.app_entrypoint_file, enable_app_server=app_spec.enable_app_server, From 1ea24ca9764d741ae4e6ef23db0cee58ecca61dc Mon Sep 17 00:00:00 2001 From: Ethan Harris Date: Fri, 11 Nov 2022 20:42:51 +0000 Subject: [PATCH 5/5] Mypy --- src/lightning_app/utilities/packaging/app_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning_app/utilities/packaging/app_config.py b/src/lightning_app/utilities/packaging/app_config.py index f4791eebabdaf..c3e44159ffb4e 100644 --- a/src/lightning_app/utilities/packaging/app_config.py +++ b/src/lightning_app/utilities/packaging/app_config.py @@ -47,7 +47,7 @@ def load_from_dir(cls, directory: Union[str, pathlib.Path]) -> "AppConfig": return cls.load_from_file(pathlib.Path(directory, _APP_CONFIG_FILENAME)) -def _get_config_file(source_path: Union[str, pathlib.Path]) -> Optional[pathlib.Path]: +def _get_config_file(source_path: Union[str, pathlib.Path]) -> pathlib.Path: """Get the Lightning app config file '.lightning' at the given source path. Args: