diff --git a/src/lightning/app/runners/cloud.py b/src/lightning/app/runners/cloud.py index 76cad0bc55b3a..ca5865e8d4296 100644 --- a/src/lightning/app/runners/cloud.py +++ b/src/lightning/app/runners/cloud.py @@ -183,26 +183,8 @@ def open(self, name: str, cluster_id: Optional[str] = None): if getattr(run, "cluster_id", None): print(f"Running on {run.cluster_id}") - # TODO: We shouldn't need to create an instance here - if existing_run_instance is not None: - run_instance = self._api_transfer_run_instance( - project.project_id, - run.id, - existing_run_instance.id, - V1LightningappInstanceState.STOPPED, - ) - else: - run_instance = self._api_create_run_instance( - cluster_id, - project.project_id, - cloudspace_name, - cloudspace_id, - run.id, - V1LightningappInstanceState.STOPPED, - ) - if "PYTEST_CURRENT_TEST" not in os.environ: - click.launch(self._get_app_url(project, cloudspace_name, run_instance, "code", needs_credits)) + click.launch(self._get_cloudspace_url(project, cloudspace_name, "code", needs_credits)) except ApiException as e: logger.error(e.body) @@ -383,9 +365,7 @@ def dispatch( # TODO: Remove testing dependency, but this would open a tab for each test... if open_ui and "PYTEST_CURRENT_TEST" not in os.environ: click.launch( - self._get_app_url( - project, cloudspace_name, run_instance, "logs" if run.is_headless else "web-ui", needs_credits - ) + self._get_app_url(project, run_instance, "logs" if run.is_headless else "web-ui", needs_credits) ) except ApiException as e: logger.error(e.body) @@ -1007,10 +987,24 @@ def _print_specs(run_body: CloudspaceIdRunsBody, print_format: str) -> None: requirements_path = getattr(getattr(run_body.image_spec, "dependency_file_info", ""), "path", "") logger.info(f"requirements_path: {requirements_path}") + def _get_cloudspace_url( + self, project: V1Membership, cloudspace_name: str, tab: str, need_credits: bool = False + ) -> str: + user = self.backend.client.auth_service_get_user() + action = "?action=add_credits" if need_credits else "" + paths = [ + user.username, + project.name, + "apps", + cloudspace_name, + tab, + ] + path = "/".join([quote(path, safe="") for path in paths]) + return f"{get_lightning_cloud_url()}/{path}{action}" + def _get_app_url( self, project: V1Membership, - cloudspace_name: str, run_instance: Externalv1LightningappInstance, tab: str, need_credits: bool = False, @@ -1021,8 +1015,8 @@ def _get_app_url( paths = [ user.username, project.name, - "apps", - cloudspace_name, + "jobs", + run_instance.name, tab, ] else: @@ -1032,5 +1026,5 @@ def _get_app_url( run_instance.id, tab, ] - path = quote("/".join([path.replace(" ", "_").replace("/", "~") for path in paths])) + path = "/".join([quote(path, safe="") for path in paths]) return f"{get_lightning_cloud_url()}/{path}{action}" diff --git a/tests/tests_app/runners/test_cloud.py b/tests/tests_app/runners/test_cloud.py index 4dae06cd3d0d6..1b10b0e3607b7 100644 --- a/tests/tests_app/runners/test_cloud.py +++ b/tests/tests_app/runners/test_cloud.py @@ -9,7 +9,6 @@ import pytest from lightning_cloud.openapi import ( - Body4, CloudspaceIdRunsBody, Externalv1Cluster, Externalv1LightningappInstance, @@ -1422,9 +1421,6 @@ def test_open(self, monkeypatch): mock_client.cloud_space_service_create_cloud_space.return_value = V1CloudSpace(id="cloudspace_id") mock_client.cloud_space_service_create_lightning_run.return_value = V1LightningRun(id="run_id") - mock_client.cloud_space_service_create_lightning_run_instance.return_value = Externalv1LightningappInstance( - id="instance_id" - ) mock_client.cluster_service_list_clusters.return_value = V1ListClustersResponse([Externalv1Cluster(id="test")]) cloud_backend = mock.MagicMock() @@ -1445,9 +1441,6 @@ def test_open(self, monkeypatch): cloudspace_id="cloudspace_id", body=mock.ANY, ) - mock_client.cloud_space_service_create_lightning_run_instance.assert_called_once_with( - project_id="test-project-id", cloudspace_id="cloudspace_id", id="run_id", body=mock.ANY - ) assert mock_client.cloud_space_service_create_cloud_space.call_args.kwargs["body"].name == "test_space" @@ -1565,10 +1558,6 @@ def test_reopen(self, monkeypatch, capsys): body=mock.ANY, ) - mock_client.lightningapp_instance_service_update_lightningapp_instance_release.assert_called_once_with( - project_id="test-project-id", id="instance_id", body=Body4(release_id="run_id") - ) - out, _ = capsys.readouterr() assert "will not overwrite the files in your CloudSpace." in out @@ -2012,12 +2001,11 @@ def run(self): @pytest.mark.parametrize( - "project, cloudspace_name, run_instance, user, tab, lightning_cloud_url, expected_url", + "project, run_instance, user, tab, lightning_cloud_url, expected_url", [ # Old style ( V1Membership(), - "any", Externalv1LightningappInstance(id="test-app-id"), V1GetUserResponse(username="tester", features=V1UserFeatures()), "logs", @@ -2026,7 +2014,6 @@ def run(self): ), ( V1Membership(), - "any", Externalv1LightningappInstance(id="test-app-id"), V1GetUserResponse(username="tester", features=V1UserFeatures()), "logs", @@ -2036,25 +2023,58 @@ def run(self): # New style ( V1Membership(name="tester's project"), - "test/app", - Externalv1LightningappInstance(), + Externalv1LightningappInstance(name="test/job"), V1GetUserResponse(username="tester", features=V1UserFeatures(project_selector=True)), "logs", "https://lightning.ai", - "https://lightning.ai/tester/tester%27s_project/apps/test~app/logs", + "https://lightning.ai/tester/tester%27s%20project/jobs/test%2Fjob/logs", ), ( V1Membership(name="tester's project"), - "test/app", - Externalv1LightningappInstance(), + Externalv1LightningappInstance(name="test/job"), V1GetUserResponse(username="tester", features=V1UserFeatures(project_selector=True)), "logs", + "https://localhost:9800", + "https://localhost:9800/tester/tester%27s%20project/jobs/test%2Fjob/logs", + ), + ], +) +def test_get_app_url(monkeypatch, project, run_instance, user, tab, lightning_cloud_url, expected_url): + mock_client = mock.MagicMock() + mock_client.auth_service_get_user.return_value = user + cloud_backend = mock.MagicMock(client=mock_client) + monkeypatch.setattr(backends, "CloudBackend", mock.MagicMock(return_value=cloud_backend)) + + runtime = CloudRuntime() + + with mock.patch( + "lightning.app.runners.cloud.get_lightning_cloud_url", mock.MagicMock(return_value=lightning_cloud_url) + ): + assert runtime._get_app_url(project, run_instance, tab) == expected_url + + +@pytest.mark.parametrize( + "user, project, cloudspace_name, tab, lightning_cloud_url, expected_url", + [ + ( + V1GetUserResponse(username="tester", features=V1UserFeatures()), + V1Membership(name="default-project"), + "test/cloudspace", + "code", + "https://lightning.ai", + "https://lightning.ai/tester/default-project/apps/test%2Fcloudspace/code", + ), + ( + V1GetUserResponse(username="tester", features=V1UserFeatures()), + V1Membership(name="Awesome Project"), + "The Best CloudSpace ever", + "web-ui", "http://localhost:9800", - "http://localhost:9800/tester/tester%27s_project/apps/test~app/logs", + "http://localhost:9800/tester/Awesome%20Project/apps/The%20Best%20CloudSpace%20ever/web-ui", ), ], ) -def test_get_app_url(monkeypatch, project, cloudspace_name, run_instance, user, tab, lightning_cloud_url, expected_url): +def test_get_cloudspace_url(monkeypatch, user, project, cloudspace_name, tab, lightning_cloud_url, expected_url): mock_client = mock.MagicMock() mock_client.auth_service_get_user.return_value = user cloud_backend = mock.MagicMock(client=mock_client) @@ -2065,4 +2085,4 @@ def test_get_app_url(monkeypatch, project, cloudspace_name, run_instance, user, with mock.patch( "lightning.app.runners.cloud.get_lightning_cloud_url", mock.MagicMock(return_value=lightning_cloud_url) ): - assert runtime._get_app_url(project, cloudspace_name, run_instance, tab) == expected_url + assert runtime._get_cloudspace_url(project, cloudspace_name, tab) == expected_url