[App] fix lightning open command & better redirects (#16794)

* fix(app): URLs, create run on app run

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 2 people

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}"

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