diff --git a/projects/orquestra-sdk/CHANGELOG.md b/projects/orquestra-sdk/CHANGELOG.md index b7b02ae34..24450e693 100644 --- 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 index c5e0d775e..aaa892ae7 100644 --- 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="") # 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 index 277256a0f..c075866d2 100644 --- 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 index 40ba7dd8d..38d851a6e 100644 --- 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 index 5bee2bbdd..5653e93da 100644 --- 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: diff --git a/projects/orquestra-sdk/tests/sdk/driver/test_client.py b/projects/orquestra-sdk/tests/sdk/driver/test_client.py index 693ba40ea..0445dce57 100644 --- a/projects/orquestra-sdk/tests/sdk/driver/test_client.py +++ b/projects/orquestra-sdk/tests/sdk/driver/test_client.py @@ -459,24 +459,12 @@ def endpoint_mocker(endpoint_mocker_base, base_uri: str): ) @staticmethod - @pytest.mark.parametrize( - "project,params", - [ - (None, {}), - ( - ProjectRef(workspace_id="a", project_id="b"), - {"workspaceId": "a", "projectId": "b"}, - ), - ], - ) def test_params_encoding( mask_sdk_version, endpoint_mocker, client: DriverClient, workflow_def_id, workflow_def: WorkflowDef, - project, - params, ): """ Verifies that params are correctly sent to the server. @@ -487,14 +475,18 @@ def test_params_encoding( responses.matchers.json_params_matcher( workflow_def.model_dump() ), - responses.matchers.query_param_matcher(params), + responses.matchers.query_param_matcher( + {"workspaceId": "a", "projectId": "b"} + ), ], # Based on: # https://github.com/zapatacomputing/workflow-driver/blob/2b353476d5b0161da31584533be208611a131bdc/openapi/src/resources/workflow-definitions.yaml#L42 status=201, ) - client.create_workflow_def(workflow_def, project) + client.create_workflow_def( + workflow_def, ProjectRef(workspace_id="a", project_id="b") + ) # The assertion is done by mocked_responses @@ -518,7 +510,9 @@ def test_sets_auth( json=resp_mocks.make_create_wf_def_response(id_=workflow_def_id), ) - client.create_workflow_def(workflow_def, None) + client.create_workflow_def( + workflow_def, ProjectRef(workspace_id="a", project_id="b") + ) # The assertion is done by mocked_responses @@ -536,7 +530,9 @@ def test_invalid_definition( ) with pytest.raises(_exceptions.InvalidWorkflowDef): - client.create_workflow_def(workflow_def, None) + client.create_workflow_def( + workflow_def, ProjectRef(workspace_id="a", project_id="b") + ) @staticmethod def test_unauthorized( @@ -548,7 +544,9 @@ def test_unauthorized( ) with pytest.raises(_exceptions.InvalidTokenError): - client.create_workflow_def(workflow_def, None) + client.create_workflow_def( + workflow_def, ProjectRef(workspace_id="a", project_id="b") + ) @staticmethod def test_forbidden( @@ -561,7 +559,9 @@ def test_forbidden( ) with pytest.raises(_exceptions.ForbiddenError): - _ = client.create_workflow_def(workflow_def, None) + _ = client.create_workflow_def( + workflow_def, ProjectRef(workspace_id="a", project_id="b") + ) @staticmethod def test_unknown_error( @@ -574,7 +574,9 @@ def test_unknown_error( ) with pytest.raises(_exceptions.UnknownHTTPError): - _ = client.create_workflow_def(workflow_def, None) + _ = client.create_workflow_def( + workflow_def, ProjectRef(workspace_id="a", project_id="b") + ) class TestDelete: @staticmethod