GSK-2887 Upload tests with project when uploading a suite

giskard/core/savable.py

@@ -53,11 +53,8 @@ def _get_name(cls) -> str:
         return f"{cls.__class__.__name__.lower()}s"
 
     @classmethod
-    def _get_meta_endpoint(cls, uuid: str, project_key: Optional[str]) -> str:
-        if project_key is None:
-            return posixpath.join(cls._get_name(), uuid)
-        else:
-            return posixpath.join("project", project_key, cls._get_name(), uuid)
+    def _get_meta_endpoint(cls, uuid: str, project_key: str) -> str:
+        return posixpath.join("project", project_key, cls._get_name(), uuid)
 
     def _save_meta_locally(self, local_dir):
         with open(Path(local_dir) / "meta.yaml", "w") as f:
@@ -77,7 +74,7 @@ def _load_meta_locally(cls, local_dir, uuid: str) -> Optional[SMT]:
     def upload(
         self,
         client: GiskardClient,
-        project_key: Optional[str] = None,
+        project_key: str,
         uploaded_dependencies: Optional[Set["Artifact"]] = None,
     ) -> str:
         """
@@ -114,7 +111,7 @@ def upload(
         return self.meta.uuid
 
     @classmethod
-    def download(cls, uuid: str, client: Optional[GiskardClient], project_key: Optional[str]) -> "Artifact":
+    def download(cls, uuid: str, client: Optional[GiskardClient], project_key: str) -> "Artifact":
         """
         Downloads the artifact from the Giskard hub or retrieves it from the local cache.
 

giskard/core/suite.py

@@ -431,7 +431,7 @@ def to_dto(self, client: GiskardClient, project_key: str, uploaded_uuid_status:
 
         return SuiteTestDTO(
             id=self.suite_test_id,
-            testUuid=self.giskard_test.upload(client),
+            testUuid=self.giskard_test.upload(client, project_key),
             functionInputs=params,
             displayName=self.display_name,
         )
@@ -935,7 +935,7 @@ def download(cls, client: GiskardClient, project_key: str, suite_id: int) -> "Su
         suite.project_key = project_key
 
         for test_json in suite_dto.tests:
-            test = GiskardTest.download(test_json.testUuid, client, None)
+            test = GiskardTest.download(test_json.testUuid, client, project_key)
             test_arguments = parse_function_arguments(client, project_key, test_json.functionInputs.values())
             suite.add_test(test(**test_arguments), suite_test_id=test_json.id)
 

tests/communications/test_websocket_actor.py

@@ -430,12 +430,10 @@ def head_slice(df: pd.DataFrame) -> pd.DataFrame:
 
 
 # FIXME: Internal worker cannot yet load callable due to yaml deserialization issue
-@pytest.mark.parametrize("callable_under_project", [False, True])
-def test_websocket_actor_dataset_processing_head_slicing_with_cache(callable_under_project, request):
+def test_websocket_actor_dataset_processing_head_slicing_with_cache(request):
     dataset: Dataset = request.getfixturevalue("enron_data")
 
     project_key = str(uuid.uuid4())  # Use a UUID to separate the resources used by the tests
-    callable_function_project_key = project_key if callable_under_project else None
 
     with utils.MockedProjectCacheDir():
         # Prepare dataset
@@ -448,7 +446,7 @@ def test_websocket_actor_dataset_processing_head_slicing_with_cache(callable_und
             functions=[
                 websocket.DatasetProcessingFunction(
                     slicingFunction=websocket.ArtifactRef(
-                        project_key=callable_function_project_key,
+                        project_key=project_key,
                         id=head_slice.meta.uuid,
                     )
                 )
@@ -458,8 +456,8 @@ def test_websocket_actor_dataset_processing_head_slicing_with_cache(callable_und
             # Prepare URL for meta info
             cf = head_slice
             # The slicing function will be loaded from the current module, without further requests
-            utils.register_uri_for_artifact_meta_info(mr, cf, project_key=callable_function_project_key)
-            utils.register_uri_for_artifact_info(mr, cf, project_key=callable_function_project_key)
+            utils.register_uri_for_artifact_meta_info(mr, cf, project_key=project_key)
+            utils.register_uri_for_artifact_info(mr, cf, project_key=project_key)
 
             # The dataset can be then loaded from the cache, without further requests
             utils.register_uri_for_dataset_meta_info(mr, dataset, project_key)
@@ -481,16 +479,14 @@ def do_nothing(row):
 
 
 # FIXME: Internal worker cannot yet load callable due to yaml deserialization issue
-@pytest.mark.parametrize("callable_under_project", [False, True])
-def test_websocket_actor_dataset_processing_do_nothing_transform_with_cache(callable_under_project, request):
+def test_websocket_actor_dataset_processing_do_nothing_transform_with_cache(request):
     dataset: Dataset = request.getfixturevalue("enron_data")
 
     project_key = str(uuid.uuid4())  # Use a UUID to separate the resources used by the tests
 
     with utils.MockedProjectCacheDir():
         # Prepare dataset
         utils.local_save_dataset_under_giskard_home_cache(dataset)
-        callable_function_project_key = project_key if callable_under_project else None
 
         do_nothing.meta.uuid = str(uuid.uuid4())
 
@@ -499,7 +495,7 @@ def test_websocket_actor_dataset_processing_do_nothing_transform_with_cache(call
             functions=[
                 websocket.DatasetProcessingFunction(
                     transformationFunction=websocket.ArtifactRef(
-                        project_key=callable_function_project_key,
+                        project_key=project_key,
                         id=do_nothing.meta.uuid,
                     )
                 )
@@ -509,8 +505,8 @@ def test_websocket_actor_dataset_processing_do_nothing_transform_with_cache(call
             # Prepare URL for meta info
             cf = do_nothing
             # The slicing function will be loaded from the current module, without further requests
-            utils.register_uri_for_artifact_meta_info(mr, cf, project_key=callable_function_project_key)
-            utils.register_uri_for_artifact_info(mr, cf, project_key=callable_function_project_key)
+            utils.register_uri_for_artifact_meta_info(mr, cf, project_key=project_key)
+            utils.register_uri_for_artifact_info(mr, cf, project_key=project_key)
 
             # The dataset can be then loaded from the cache, without further requests
             utils.register_uri_for_dataset_meta_info(mr, dataset, project_key)

tests/communications/test_websocket_actor_tests.py

@@ -40,12 +40,10 @@ def my_simple_test_error():
 def test_websocket_actor_run_ad_hoc_test_no_debug(debug):
     with utils.MockedProjectCacheDir():
         params = websocket.RunAdHocTestParam(
-            testUuid=my_simple_test.meta.uuid,
-            arguments=[],
-            debug=debug,
+            testUuid=my_simple_test.meta.uuid, arguments=[], debug=debug, projectKey="project_key"
         )
         with utils.MockedClient(mock_all=False) as (client, mr):
-            utils.register_uri_for_artifact_meta_info(mr, my_simple_test, None)
+            utils.register_uri_for_artifact_meta_info(mr, my_simple_test, "project_key")
 
             reply = listener.run_ad_hoc_test(client=client, params=params)
             assert isinstance(reply, websocket.RunAdHocTest)
@@ -131,34 +129,6 @@ def test_websocket_actor_run_ad_hoc_test_legacy_no_client(enron_data: Dataset):
             listener.run_ad_hoc_test(client=None, params=params)
 
 
-def test_websocket_actor_run_ad_hoc_test_legacy_no_project_key(enron_data: Dataset):
-    project_key = str(uuid.uuid4())
-
-    with utils.MockedProjectCacheDir():
-        utils.local_save_dataset_under_giskard_home_cache(enron_data)
-
-        params = websocket.RunAdHocTestParam(
-            testUuid=my_simple_test_legacy_debug.meta.uuid,
-            arguments=[
-                websocket.FuncArgument(
-                    name="dataset",
-                    none=False,
-                    dataset=websocket.ArtifactRef(
-                        project_key=project_key,
-                        id=str(enron_data.id),
-                    ),
-                ),
-            ],
-            debug=True,
-        )
-
-        with utils.MockedClient(mock_all=False) as (client, mr), pytest.raises(ValueError):
-            utils.register_uri_for_artifact_meta_info(mr, my_simple_test_legacy_debug, None)
-            utils.register_uri_for_dataset_meta_info(mr, enron_data, project_key)
-
-            listener.run_ad_hoc_test(client=client, params=params)
-
-
 @test
 def my_simple_test_debug(dataset: Dataset, debug: bool = False):
     return GiskardTestResult(passed=False, output_ds=[dataset.slice(lambda df: df.head(1), row_level=False)])
@@ -183,9 +153,10 @@ def test_websocket_actor_run_ad_hoc_test_debug(enron_data: Dataset):
                 ),
             ],
             debug=True,
+            projectKey=project_key,
         )
         with utils.MockedClient(mock_all=False) as (client, mr):
-            utils.register_uri_for_artifact_meta_info(mr, my_simple_test_debug, None)
+            utils.register_uri_for_artifact_meta_info(mr, my_simple_test_debug, project_key)
             utils.register_uri_for_dataset_meta_info(mr, enron_data, project_key)
             utils.register_uri_for_any_dataset_artifact_info_upload(mr, register_files=True)
 
@@ -246,9 +217,10 @@ def test_websocket_actor_run_ad_hoc_test_debug_multiple_datasets(enron_data: Dat
                 ),
             ],
             debug=True,
+            projectKey=project_key,
         )
         with utils.MockedClient(mock_all=False) as (client, mr):
-            utils.register_uri_for_artifact_meta_info(mr, my_simple_test_debug_multiple_datasets, None)
+            utils.register_uri_for_artifact_meta_info(mr, my_simple_test_debug_multiple_datasets, project_key)
             utils.register_uri_for_dataset_meta_info(mr, enron_data, project_key)
             utils.register_uri_for_dataset_meta_info(mr, dataset2, project_key)
             utils.register_uri_for_any_dataset_artifact_info_upload(mr, register_files=True)

tests/core/test_suite.py

@@ -21,20 +21,21 @@ def my_test(model: BaseModel):
 
 
 def test_save_suite_with_artifact_error():
+    project_key = "project_key"
     model = FailingModel(model_type="regression")
     regex_model_name = str(model).replace("(", "\\(").replace(")", "\\)")
 
     with MockedClient() as (client, mr), pytest.warns(
         UserWarning,
         match=f"Failed to upload {regex_model_name} used in the test suite. The test suite will be partially uploaded.",
     ):
-        utils.register_uri_for_artifact_meta_info(mr, my_test, None)
+        utils.register_uri_for_artifact_meta_info(mr, my_test, project_key)
         mr.register_uri(
             method=requests_mock.POST,
-            url="http://giskard-host:12345/api/v2/testing/project/titanic/suites",
+            url="http://giskard-host:12345/api/v2/testing/project/project_key/suites",
             json={"id": 1, "tests": [{"id": 2}]},
         )
 
         test_suite = Suite().add_test(my_test, model=model)
 
-        test_suite.upload(client, "titanic")
+        test_suite.upload(client, project_key)

tests/test_artifact_download.py

@@ -26,6 +26,7 @@
 )
 
 BASE_CLIENT_URL = "http://giskard-host:12345/api/v2"
+PROJECT_KEY = "project_key"
 
 
 # Define a test function
@@ -109,12 +110,12 @@ def test_download_callable_function(cf: Artifact):
                     "name": f"fake_{cf._get_name()}",
                 }
             )
-            url = get_url_for_artifact_meta_info(cf, project_key=None)
+            url = get_url_for_artifact_meta_info(cf, PROJECT_KEY)
             mr.register_uri(method=requests_mock.GET, url=url, json=meta_info)
             requested_urls.append(url)
 
             # Register for Artifact info
-            requested_urls.extend(register_uri_for_artifact_info(mr, cf, project_key=None))
+            requested_urls.extend(register_uri_for_artifact_info(mr, cf, PROJECT_KEY))
 
             # Register for Artifacts content
             artifacts_base_url = get_url_for_artifacts_base(cf)
@@ -128,7 +129,7 @@ def test_download_callable_function(cf: Artifact):
                     requested_urls.append(posixpath.join(artifacts_base_url, file))
 
             # Download: should not call load_artifact to request and download
-            download_cf = cf.__class__.download(uuid=cf.meta.uuid, client=client, project_key=None)
+            download_cf = cf.__class__.download(uuid=cf.meta.uuid, client=client, project_key=PROJECT_KEY)
 
             for requested_url in requested_urls:
                 assert is_url_requested(mr.request_history, requested_url)
@@ -149,17 +150,17 @@ def test_download_callable_function(cf: Artifact):
         do_nothing,  # Transformation
     ],
 )
-def test_download_global_callable_function_from_module(cf: Artifact):
+def test_download_callable_function_from_module(cf: Artifact):
     with MockedClient(mock_all=False) as (client, mr):
         cf.meta.uuid = str(uuid.uuid4())  # Regenerate a UUID to ensure not loading from registry
         cache_dir = get_local_cache_callable_artifact(artifact=cf)
 
         requested_urls = []
         # Prepare global URL
-        requested_urls.extend(register_uri_for_artifact_meta_info(mr, cf, project_key=None))
+        requested_urls.extend(register_uri_for_artifact_meta_info(mr, cf, project_key=PROJECT_KEY))
 
         # Download: should not call load_artifact to request and download
-        download_cf = cf.__class__.download(uuid=cf.meta.uuid, client=client, project_key=None)
+        download_cf = cf.__class__.download(uuid=cf.meta.uuid, client=client, project_key=PROJECT_KEY)
 
         for requested_url in requested_urls:
             assert is_url_requested(mr.request_history, requested_url)
@@ -180,7 +181,7 @@ def test_download_global_callable_function_from_module(cf: Artifact):
         do_nothing,  # Transformation
     ],
 )
-def test_download_global_callable_function_from_cache(cf: Artifact):
+def test_download_callable_function_from_cache(cf: Artifact):
     with MockedClient(mock_all=False) as (client, mr):
         cf.meta.uuid = str(uuid.uuid4())  # Regenerate a UUID
         cache_dir = get_local_cache_callable_artifact(artifact=cf)
@@ -190,10 +191,10 @@ def test_download_global_callable_function_from_cache(cf: Artifact):
         assert (cache_dir / CALLABLE_FUNCTION_PKL_CACHE).exists()
         assert (cache_dir / CALLABLE_FUNCTION_META_CACHE).exists()
 
-        requested_urls = register_uri_for_artifact_meta_info(mr, cf, project_key=None)
+        requested_urls = register_uri_for_artifact_meta_info(mr, cf, project_key=PROJECT_KEY)
 
         # Download: should not call load_artifact to request and download
-        download_cf = cf.__class__.download(uuid=cf.meta.uuid, client=client, project_key=None)
+        download_cf = cf.__class__.download(uuid=cf.meta.uuid, client=client, project_key=PROJECT_KEY)
 
         for requested_url in requested_urls:
             assert is_url_requested(mr.request_history, requested_url)

tests/test_programmatic.py

@@ -434,7 +434,7 @@ def test_download_suite_run_and_upload_results():
                 "projectKey": "test_project",
             },
         )
-        register_uri_for_artifact_meta_info(mr, test_auc)
+        register_uri_for_artifact_meta_info(mr, test_auc, "test_project")
 
         mr.register_uri(requests_mock.GET, UPLOAD_RESULTS_URL, json={})
 

tests/test_upload.py

@@ -17,6 +17,7 @@
 )
 
 model_name = "uploaded model"
+PROJECT_KEY = "project_key"
 
 
 def test_upload_df(diabetes_dataset: Dataset, diabetes_dataset_with_target: Dataset):
@@ -151,7 +152,7 @@ def test_upload_callable_function(cf: Artifact):
         + "/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/.*"
     )
     with MockedClient() as (client, mr):
-        cf.upload(client=client, project_key=None)
+        cf.upload(client=client, project_key=PROJECT_KEY)
         # Check local cache
         cache_dir = get_local_cache_callable_artifact(artifact=cf)
         assert (cache_dir / CALLABLE_FUNCTION_PKL_CACHE).exists()

tests/utils.py

@@ -264,12 +264,8 @@ def mock_model_meta_info(model: BaseModel, project_key: str):
     return model_meta_info.dict()
 
 
-def get_url_for_artifact_meta_info(cf: Artifact, project_key: Optional[str] = None):
-    return (
-        posixpath.join(CLIENT_BASE_URL, "project", project_key, cf._get_name(), cf.meta.uuid)
-        if project_key
-        else posixpath.join(CLIENT_BASE_URL, cf._get_name(), cf.meta.uuid)
-    )
+def get_url_for_artifact_meta_info(cf: Artifact, project_key: str):
+    return posixpath.join(CLIENT_BASE_URL, "project", project_key, cf._get_name(), cf.meta.uuid)
 
 
 def get_url_for_artifacts_base(cf: Artifact):
@@ -284,7 +280,7 @@ def get_url_for_model(model: BaseModel, project_key: str):
     return posixpath.join(CLIENT_BASE_URL, "project", project_key, "models", str(model.id))
 
 
-def register_uri_for_artifact_meta_info(mr: requests_mock.Mocker, cf: Artifact, project_key: Optional[str] = None):
+def register_uri_for_artifact_meta_info(mr: requests_mock.Mocker, cf: Artifact, project_key: str):
     url = get_url_for_artifact_meta_info(cf, project_key)
     # Fixup the differences from Backend
     meta_info = fixup_mocked_artifact_meta_version(cf.meta.to_json())