Feat!: remove ability to submit workflows without workspace (#451)

# The problem WFs without workspaces were deprecated some time ago https://zapatacomputing.atlassian.net/browse/ORQSDK-1065 # This PR's solution Remove the ability to submit workflows without project to CE # Checklist _Check that this PR satisfies the following items:_ - [x] Tests have been added for new features/changed behavior (if no new features have been added, check the box). - [x] The [changelog file](CHANGELOG.md) has been updated with a user-readable description of the changes (if the change isn't visible to the user in any way, check the box). - [x] The PR's title is prefixed with `<feat/fix/chore/imp[rovement]/int[ernal]/docs>[!]:` - [x] The PR is linked to a JIRA ticket (if there's no suitable ticket, check the box).
zapata-engineering · Oct 8, 2024 · d33b552 · d33b552
1 parent 8f5b4e8
commit d33b552
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 135 deletions.
diff --git a/projects/orquestra-sdk/CHANGELOG.md b/projects/orquestra-sdk/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 🚨 *Breaking Changes*
 
+* Setting `workspace_id` is now required when submitting workflow to remote clusters
+
 🔥 *Features*
 
 * Allow to set `int` as GPU number in task resources.

diff --git a/projects/orquestra-sdk/docs/examples/tests/test_remote.py b/projects/orquestra-sdk/docs/examples/tests/test_remote.py
@@ -48,7 +48,7 @@ def execute_workflow():
         workflow = hello_orquestra_wf()
         remote_config = sdk.RuntimeConfig.load(config_name="<paste your config name>")
         # Start the workflow
-        workflow_run = workflow.run(remote_config)
+        workflow_run = workflow.run(remote_config, workspace_id="x", project_id="y")
 
         # Get the workflow run ID
         print(workflow_run.run_id)

diff --git a/projects/orquestra-sdk/src/orquestra/sdk/_client/_base/_driver/_ce_runtime.py b/projects/orquestra-sdk/src/orquestra/sdk/_client/_base/_driver/_ce_runtime.py
@@ -180,11 +180,10 @@ def create_workflow_run(
             the workflow run ID.
         """
         if project is None or project.workspace_id is None:
-            warnings.warn(
-                "Please specify workspace ID directly for submitting CE workflows."
-                " Support for default workspace will be removed in the next release",
-                category=PendingDeprecationWarning,
+            raise ValueError(
+                "Please specify workspace ID directly for " "submitting CE workflows."
             )
+
         _check_token_validity(self._token)
 
         max_invocation_resources = _get_max_resources(workflow_def)
@@ -252,15 +251,10 @@ def create_workflow_run(
                 "Unable to start the workflow run."
             ) from e
         except _exceptions.ForbiddenError as e:
-            if project:
-                raise exceptions.ProjectInvalidError(
-                    f"Unable to start the workflow run "
-                    f"invalid workspace: {project.workspace_id}"
-                ) from e
-            else:
-                raise exceptions.UnauthorizedError(
-                    "Unable to start the workflow run "
-                ) from e
+            raise exceptions.ProjectInvalidError(
+                f"Unable to start the workflow run "
+                f"invalid workspace: {project.workspace_id}"
+            ) from e
         except _exceptions.InvalidTokenError as e:
             raise exceptions.UnauthorizedError(
                 "Unable to start the workflow run "

diff --git a/projects/orquestra-sdk/src/orquestra/sdk/_client/_base/_driver/_client.py b/projects/orquestra-sdk/src/orquestra/sdk/_client/_base/_driver/_client.py
@@ -249,7 +249,7 @@ def _delete(self, uri: str) -> requests.Response:
     def create_workflow_def(
         self,
         workflow_def: WorkflowDef,
-        project: Optional[ProjectRef],
+        project: ProjectRef,
     ) -> _models.WorkflowDefID:
         """Stores a workflow definition for future submission.
 
@@ -270,14 +270,10 @@ def create_workflow_def(
         Returns:
             the WorkflowDefID associated with the stored definition
         """
-        query_params = (
-            _models.CreateWorkflowDefsRequest(
-                workspaceId=project.workspace_id,
-                projectId=project.project_id,
-            ).model_dump()
-            if project
-            else None
-        )
+        query_params = _models.CreateWorkflowDefsRequest(
+            workspaceId=project.workspace_id,
+            projectId=project.project_id,
+        ).model_dump()
         resp = self._post(
             self._uri_provider.uri_for("create_workflow_def"),
             body_params=workflow_def.model_dump(),

diff --git a/projects/orquestra-sdk/tests/sdk/driver/test_ce_runtime.py b/projects/orquestra-sdk/tests/sdk/driver/test_ce_runtime.py
@@ -96,14 +96,15 @@ def test_happy_path(
         monkeypatch.setattr(_traversal, "_gen_id_hash", lambda *_: 0)
 
         # When
-        with pytest.warns(PendingDeprecationWarning):
-            wf_run_id = runtime.create_workflow_run(
-                my_workflow.model, project=None, dry_run=False
-            )
+        wf_run_id = runtime.create_workflow_run(
+            my_workflow.model,
+            project=ProjectRef(workspace_id="a", project_id="b"),
+            dry_run=False,
+        )
 
         # Then
         mocked_client.create_workflow_def.assert_called_once_with(
-            my_workflow.model, None
+            my_workflow.model, ProjectRef(workspace_id="a", project_id="b")
         )
         mocked_client.create_workflow_run.assert_called_once_with(
             workflow_def_id,
@@ -129,12 +130,11 @@ def test_with_memory(
             mocked_client.create_workflow_run.return_value = workflow_run_id
 
             # When
-            with pytest.warns(PendingDeprecationWarning):
-                _ = runtime.create_workflow_run(
-                    workflow_parametrised_with_resources(memory="10Gi").model,
-                    None,
-                    dry_run=False,
-                )
+            _ = runtime.create_workflow_run(
+                workflow_parametrised_with_resources(memory="10Gi").model,
+                ProjectRef(workspace_id="a", project_id="b"),
+                dry_run=False,
+            )
 
             # Then
             mocked_client.create_workflow_run.assert_called_once_with(
@@ -156,12 +156,11 @@ def test_with_cpu(
             mocked_client.create_workflow_run.return_value = workflow_run_id
 
             # When
-            with pytest.warns(PendingDeprecationWarning):
-                _ = runtime.create_workflow_run(
-                    workflow_parametrised_with_resources(cpu="1000m").model,
-                    None,
-                    dry_run=False,
-                )
+            _ = runtime.create_workflow_run(
+                workflow_parametrised_with_resources(cpu="1000m").model,
+                ProjectRef(workspace_id="a", project_id="b"),
+                dry_run=False,
+            )
 
             # Then
             mocked_client.create_workflow_run.assert_called_once_with(
@@ -183,12 +182,11 @@ def test_with_gpu(
             mocked_client.create_workflow_run.return_value = workflow_run_id
 
             # When
-            with pytest.warns(PendingDeprecationWarning):
-                _ = runtime.create_workflow_run(
-                    workflow_parametrised_with_resources(gpu="1").model,
-                    None,
-                    dry_run=False,
-                )
+            _ = runtime.create_workflow_run(
+                workflow_parametrised_with_resources(gpu="1").model,
+                ProjectRef(workspace_id="a", project_id="b"),
+                dry_run=False,
+            )
 
             # Then
             mocked_client.create_workflow_run.assert_called_once_with(
@@ -210,10 +208,11 @@ def test_maximum_resource(
             mocked_client.create_workflow_run.return_value = workflow_run_id
 
             # When
-            with pytest.warns(PendingDeprecationWarning):
-                _ = runtime.create_workflow_run(
-                    workflow_with_different_resources().model, None, dry_run=False
-                )
+            _ = runtime.create_workflow_run(
+                workflow_with_different_resources().model,
+                ProjectRef(workspace_id="a", project_id="b"),
+                dry_run=False,
+            )
 
             # Then
             mocked_client.create_workflow_run.assert_called_once_with(
@@ -235,14 +234,13 @@ def test_resources_from_workflow(
             mocked_client.create_workflow_run.return_value = workflow_run_id
 
             # When
-            with pytest.warns(PendingDeprecationWarning):
-                _ = runtime.create_workflow_run(
-                    my_workflow()
-                    .with_resources(cpu="1", memory="1.5G", gpu="1", nodes=20)
-                    .model,
-                    None,
-                    dry_run=False,
-                )
+            _ = runtime.create_workflow_run(
+                my_workflow()
+                .with_resources(cpu="1", memory="1.5G", gpu="1", nodes=20)
+                .model,
+                ProjectRef(workspace_id="a", project_id="b"),
+                dry_run=False,
+            )
 
             # Then
             mocked_client.create_workflow_run.assert_called_once_with(
@@ -278,12 +276,11 @@ def wf():
         mocked_client.create_workflow_run.return_value = workflow_run_id
 
         # When
-        with pytest.warns(PendingDeprecationWarning):
-            _ = runtime.create_workflow_run(
-                wf().model,
-                None,
-                dry_run=False,
-            )
+        _ = runtime.create_workflow_run(
+            wf().model,
+            ProjectRef(workspace_id="a", project_id="b"),
+            dry_run=False,
+        )
 
         # Then
         mocked_client.create_workflow_run.assert_called_once_with(
@@ -319,12 +316,11 @@ def wf():
         mocked_client.create_workflow_run.return_value = workflow_run_id
 
         # When
-        with pytest.warns(PendingDeprecationWarning):
-            _ = runtime.create_workflow_run(
-                wf().model,
-                None,
-                dry_run=False,
-            )
+        _ = runtime.create_workflow_run(
+            wf().model,
+            ProjectRef(workspace_id="a", project_id="b"),
+            dry_run=False,
+        )
 
         # Then
         mocked_client.create_workflow_run.assert_called_once_with(
@@ -345,10 +341,11 @@ def test_invalid_wf_def(
 
             # When
             with pytest.raises(exceptions.WorkflowSyntaxError):
-                with pytest.warns(PendingDeprecationWarning):
-                    _ = runtime.create_workflow_run(
-                        my_workflow.model, None, dry_run=False
-                    )
+                _ = runtime.create_workflow_run(
+                    my_workflow.model,
+                    ProjectRef(workspace_id="a", project_id="b"),
+                    dry_run=False,
+                )
 
         def test_unknown_http(
             self, mocked_client: MagicMock, runtime: _ce_runtime.CERuntime
@@ -360,43 +357,40 @@ def test_unknown_http(
 
             # When
             with pytest.raises(_exceptions.UnknownHTTPError):
-                with pytest.warns(PendingDeprecationWarning):
-                    _ = runtime.create_workflow_run(
-                        my_workflow.model, None, dry_run=False
-                    )
+                _ = runtime.create_workflow_run(
+                    my_workflow.model,
+                    ProjectRef(workspace_id="a", project_id="b"),
+                    dry_run=False,
+                )
 
-        @pytest.mark.parametrize(
-            "failure_exc", [_exceptions.InvalidTokenError, _exceptions.ForbiddenError]
-        )
-        def test_auth_failure(
+        def test_invalid_project(
             self,
             mocked_client: MagicMock,
             runtime: _ce_runtime.CERuntime,
-            failure_exc: Exception,
         ):
             # Given
-            mocked_client.create_workflow_def.side_effect = failure_exc
+            mocked_client.create_workflow_def.side_effect = _exceptions.ForbiddenError
 
             # When
-            with pytest.raises(exceptions.UnauthorizedError):
-                with pytest.warns(PendingDeprecationWarning):
-                    _ = runtime.create_workflow_run(
-                        my_workflow.model, None, dry_run=False
-                    )
+            with pytest.raises(exceptions.ProjectInvalidError):
+                _ = runtime.create_workflow_run(
+                    my_workflow.model,
+                    ProjectRef(workspace_id="a", project_id="b"),
+                    False,
+                )
 
-        def test_invalid_project(
+        def test_no_workspace(
             self,
             mocked_client: MagicMock,
             runtime: _ce_runtime.CERuntime,
         ):
             # Given
-            mocked_client.create_workflow_def.side_effect = _exceptions.ForbiddenError
 
             # When
-            with pytest.raises(exceptions.ProjectInvalidError):
+            with pytest.raises(ValueError):
                 _ = runtime.create_workflow_run(
                     my_workflow.model,
-                    ProjectRef(workspace_id="a", project_id="b"),
+                    None,
                     False,
                 )
 
@@ -418,10 +412,11 @@ def test_invalid_wf_run(
 
             # When
             with pytest.raises(exceptions.WorkflowRunNotStarted):
-                with pytest.warns(PendingDeprecationWarning):
-                    _ = runtime.create_workflow_run(
-                        my_workflow.model, None, dry_run=False
-                    )
+                _ = runtime.create_workflow_run(
+                    my_workflow.model,
+                    ProjectRef(workspace_id="a", project_id="b"),
+                    dry_run=False,
+                )
 
         @pytest.mark.parametrize("submitted_version", (None, "0.1.0"))
         @pytest.mark.parametrize(
@@ -441,10 +436,11 @@ def test_unsupported_sdk_version(
 
             # When
             with pytest.raises(exceptions.WorkflowRunNotStarted) as exc_info:
-                with pytest.warns(PendingDeprecationWarning):
-                    _ = runtime.create_workflow_run(
-                        my_workflow.model, None, dry_run=False
-                    )
+                _ = runtime.create_workflow_run(
+                    my_workflow.model,
+                    ProjectRef(workspace_id="a", project_id="b"),
+                    dry_run=False,
+                )
 
             error_message = str(exc_info.value)
             assert "This is an unsupported version of orquestra-sdk.\n" in error_message
@@ -473,29 +469,29 @@ def test_unknown_http(
 
             # When
             with pytest.raises(_exceptions.UnknownHTTPError):
-                with pytest.warns(PendingDeprecationWarning):
-                    _ = runtime.create_workflow_run(
-                        my_workflow.model, None, dry_run=False
-                    )
+                _ = runtime.create_workflow_run(
+                    my_workflow.model,
+                    ProjectRef(workspace_id="a", project_id="b"),
+                    dry_run=False,
+                )
 
-        @pytest.mark.parametrize(
-            "failure_exc", [_exceptions.InvalidTokenError, _exceptions.ForbiddenError]
-        )
         def test_auth_failure(
             self,
             mocked_client: MagicMock,
             runtime: _ce_runtime.CERuntime,
-            failure_exc: Exception,
         ):
             # Given
-            mocked_client.create_workflow_run.side_effect = failure_exc
+            mocked_client.create_workflow_run.side_effect = (
+                _exceptions.InvalidTokenError
+            )
 
             # When
             with pytest.raises(exceptions.UnauthorizedError):
-                with pytest.warns(PendingDeprecationWarning):
-                    _ = runtime.create_workflow_run(
-                        my_workflow.model, None, dry_run=False
-                    )
+                _ = runtime.create_workflow_run(
+                    my_workflow.model,
+                    ProjectRef(workspace_id="a", project_id="b"),
+                    dry_run=False,
+                )
 
 
 class TestGetWorkflowRunStatus:
@@ -2154,8 +2150,9 @@ def wf():
 
     # When
     with context as exec_info:
-        with pytest.warns(PendingDeprecationWarning):
-            runtime.create_workflow_run(wf().model, None, dry_run=False)
+        runtime.create_workflow_run(
+            wf().model, ProjectRef(workspace_id="a", project_id="b"), dry_run=False
+        )
 
     # Then
     if raises: