Merge pull request #187 from gchq/feature/BAI-341-remove-option-expor…

…t-raw-models removed export raw model build option
gchq · Nov 2, 2022 · 719d402 · 719d402
2 parents aecc3ce + 80b4ce1
commit 719d402
Show file tree

Hide file tree

Showing 12 changed files with 35 additions and 106 deletions.
diff --git a/__tests__/example_models/minimal_model/minimal_metadata.json b/__tests__/example_models/minimal_model/minimal_metadata.json
@@ -12,7 +12,6 @@
     "manager": "user"
   },
   "buildOptions": {
-    "exportRawModel": true,
     "uploadType": "Code and binaries"
   }
 }
diff --git a/__tests__/example_models/minimal_model/minimal_metadata_edit.json b/__tests__/example_models/minimal_model/minimal_metadata_edit.json
@@ -12,7 +12,6 @@
     "manager": "user"
   },
   "buildOptions": {
-    "exportRawModel": true,
     "uploadType": "Model card only"
   }
 }
diff --git a/__tests__/example_models/minimal_model/minimal_metadata_modelcard.json b/__tests__/example_models/minimal_model/minimal_metadata_modelcard.json
@@ -12,7 +12,6 @@
     "manager": "user"
   },
   "buildOptions": {
-    "exportRawModel": true,
     "uploadType": "Model card only"
   }
 }
diff --git a/__tests__/example_models/minimal_model/minimal_metadata_modelcard_new_version.json b/__tests__/example_models/minimal_model/minimal_metadata_modelcard_new_version.json
@@ -12,7 +12,6 @@
     "manager": "user"
   },
   "buildOptions": {
-    "exportRawModel": true,
     "uploadType": "Model card only"
   }
 }
diff --git a/lib/python/bailoclient/client.py b/lib/python/bailoclient/client.py
@@ -27,7 +27,6 @@
     InvalidFilePath,
     InvalidFileRequested,
     InvalidMetadata,
-    ModelFileExportNotAllowed,
     UnconnectedClient,
 )
 from .utils.utils import get_filename_and_mimetype, minimal_keys_in_dictionary
@@ -199,17 +198,13 @@ def download_model_files(
             overwrite (bool, optional): Whether to overwrite an existing folder with download. Defaults to False.
 
         Raises:
-            ModelFileExportNotAllowed: Model files are not exportable for this model.
             InvalidFileRequested: Invalid file type - must be 'code' or 'binary'
             FileExistsError: File already exists at filepath. Overwrite must be specified to overwrite.
 
         Returns:
             str: Response status code
         """
 
-        if not self.__allow_exports(deployment_uuid):
-            raise ModelFileExportNotAllowed("Files are not exportable for this model")
-
         if not file_type in ["code", "binary"]:
             raise InvalidFileRequested(
                 "Invalid file_type provided - file_type can either be 'code' or 'binary'"
@@ -225,20 +220,6 @@ def download_model_files(
             output_dir=output_dir,
         )
 
-    def __allow_exports(self, deployment_uuid: str):
-        """Check whether the model files associated with a deployment are allowed to be exported
-
-        Args:
-            deployment_uuid (str): UUID of the delpoyment
-
-        Returns:
-            bool: True is model is exportable
-        """
-        model_uuid = self.get_deployment_by_uuid(deployment_uuid)["model"]["uuid"]
-        model_card = self.get_model_card(model_uuid=model_uuid)
-
-        return model_card["currentMetadata"]["buildOptions"]["exportRawModel"]
-
     @handle_reconnect
     def get_deployment_by_uuid(self, deployment_uuid: str):
         """Get deployment by deployment UUID

diff --git a/lib/python/bailoclient/config.py b/lib/python/bailoclient/config.py
@@ -11,7 +11,7 @@
 class APIConfig(BaseSettings):
     """Configuration for the Bailo API"""
 
-    url: HttpUrl
+    url: str
     ca_verify: Union[bool, str]
 
 

diff --git a/lib/python/bailoclient/utils/exceptions.py b/lib/python/bailoclient/utils/exceptions.py
@@ -55,7 +55,3 @@ class InvalidFileRequested(Exception):
 
 class DeploymentNotFound(Exception):
     """Could not find a deployment"""
-
-
-class ModelFileExportNotAllowed(Exception):
-    """Exporting model files not allowed for this model"""
diff --git a/lib/python/tests/test_client.py b/lib/python/tests/test_client.py
@@ -15,7 +15,6 @@
     InvalidFilePath,
     InvalidFileRequested,
     InvalidMetadata,
-    ModelFileExportNotAllowed,
 )
 
 from tests.mocks.mock_api import MockAPI
@@ -275,19 +274,7 @@ def test_upload_model_is_called_with_expected_params(
     )
 
 
-@patch("bailoclient.client.Client._Client__allow_exports", return_value=False)
-def test_download_model_files_raises_error_if_model_files_are_not_exportable(
-    mock_allow_exports, mock_client
-):
-    with pytest.raises(ModelFileExportNotAllowed):
-        mock_client.download_model_files(
-            deployment_uuid="test", model_version="1", file_type="invalid"
-        )
-
-
-@patch("bailoclient.client.Client._Client__allow_exports", return_value=True)
 def test_download_model_files_raises_error_if_file_type_is_not_code_or_binary(
-    mock_allow_exports,
     mock_client,
 ):
     with pytest.raises(InvalidFileRequested):
@@ -296,19 +283,17 @@ def test_download_model_files_raises_error_if_file_type_is_not_code_or_binary(
         )
 
 
-@patch("bailoclient.client.Client._Client__allow_exports", return_value=True)
 def test_download_model_files_raises_error_if_output_dir_already_exists_and_user_has_not_specified_overwrite(
-    mock_allow_exports, mock_client, tmpdir
+    mock_client, tmpdir
 ):
     with pytest.raises(FileExistsError):
         mock_client.download_model_files(
             deployment_uuid="test", model_version="1", output_dir=str(tmpdir)
         )
 
 
-@patch("bailoclient.client.Client._Client__allow_exports", return_value=True)
 def test_download_model_files_overwrites_existing_output_dir_if_user_has_specified_overwrite(
-    mock_allow_exports, mock_client, tmpdir
+    mock_client, tmpdir
 ):
     deployment_uuid = "test"
     model_version = "1"
@@ -330,23 +315,6 @@ def test_download_model_files_overwrites_existing_output_dir_if_user_has_specifi
     )
 
 
-@patch("bailoclient.client.Client.get_deployment_by_uuid")
-@patch("bailoclient.client.Client.get_model_card")
-def test_allow_exports_returns_false_if_model_does_not_allow_exports(
-    mock_get_model_card, mock_get_deployment, mock_client
-):
-    mock_get_deployment.return_value = {"model": {"uuid": "123"}}
-    mock_get_model_card.return_value = {
-        "currentMetadata": {"buildOptions": {"exportRawModel": False}}
-    }
-
-    allow_exports = mock_client._Client__allow_exports("dep")
-
-    mock_get_deployment.mock_called_once_with("dep")
-    mock_get_model_card.assert_called_once_with(model_uuid="123")
-    assert not allow_exports
-
-
 @patch("bailoclient.client.Client.get_me", return_value=User(_id="user"))
 @patch(
     "bailoclient.client.Client.get_user_deployments",

diff --git a/server/scripts/example_schemas/minimal_upload_schema.json b/server/scripts/example_schemas/minimal_upload_schema.json
@@ -101,11 +101,6 @@
       "description": "Optional build options for a model",
       "type": "object",
       "properties": {
-        "exportRawModel": {
-          "title": "Export raw models",
-          "description": "If enabled, allow raw uploaded model files to be downloaded by deployments.",
-          "type": "boolean"
-        },
         "uploadType": {
           "type": "string",
           "title": "Upload type",

diff --git a/server/scripts/example_schemas/minimal_upload_schema_examples.json b/server/scripts/example_schemas/minimal_upload_schema_examples.json
@@ -13,7 +13,7 @@
       "manager": "user"
     },
     "buildOptions": {
-      "exportRawModel": true
+      "uploadType": "Code and binaries"
     }
   },
   {
@@ -30,7 +30,6 @@
       "manager": "user"
     },
     "buildOptions": {
-      "exportRawModel": true,
       "uploadType": "Code and binaries"
     }
   },
@@ -48,7 +47,7 @@
       "manager": "user"
     },
     "buildOptions": {
-      "exportRawModel": true
+      "uploadType": "Code and binaries"
     }
   }
 ]
diff --git a/src/RawModelExportList.tsx b/src/RawModelExportList.tsx
@@ -8,8 +8,8 @@ import Button from '@mui/material/Button'
 
 import { useGetModelById, useGetModelVersions } from '../data/model'
 import { Deployment } from '../types/interfaces'
-import EmptyBlob from './common/EmptyBlob'
 import { ModelDoc } from '../server/models/Model'
+import EmptyBlob from './common/EmptyBlob'
 
 function RawModelExportList({ deployment }: { deployment: Deployment }) {
   const modelFromDeployment: ModelDoc = deployment.model as ModelDoc
@@ -19,40 +19,35 @@ function RawModelExportList({ deployment }: { deployment: Deployment }) {
   return (
     <>
       {versions &&
-        versions.map(
-          (version: any) =>
-            version.metadata?.buildOptions?.exportRawModel && (
-              <Box key={version.version}>
-                <Box sx={{ p: 1 }}>
-                  <Box sx={{ p: 2 }}>
-                    <Typography variant='h4'>Version: {version.version}</Typography>
-                  </Box>
-                  <Stack spacing={2} direction='row' sx={{ p: 1 }}>
-                    <Button
-                      variant='contained'
-                      href={`/api/v1/deployment/${deployment.uuid}/version/${version.version}/raw/code`}
-                      target='_blank'
-                      data-test='downloadCodeFile'
-                    >
-                      Download code file
-                    </Button>
-                    <Button
-                      variant='contained'
-                      href={`/api/v1/deployment/${deployment.uuid}/version/${version.version}/raw/binary`}
-                      target='_blank'
-                      data-test='downloadBinaryFile'
-                    >
-                      Download binary file
-                    </Button>
-                  </Stack>
-                </Box>
-                <Divider orientation='horizontal' />
+        versions.map((version: any) => (
+          <Box key={version.version}>
+            <Box sx={{ p: 1 }}>
+              <Box sx={{ p: 2 }}>
+                <Typography variant='h4'>Version: {version.version}</Typography>
               </Box>
-            )
-        )}
-      {versions && versions.filter((e) => e.metadata.buildOptions?.exportRawModel).length === 0 && (
-        <EmptyBlob text='No exportable versions' />
-      )}
+              <Stack spacing={2} direction='row' sx={{ p: 1 }}>
+                <Button
+                  variant='contained'
+                  href={`/api/v1/deployment/${deployment.uuid}/version/${version.version}/raw/code`}
+                  target='_blank'
+                  data-test='downloadCodeFile'
+                >
+                  Download code file
+                </Button>
+                <Button
+                  variant='contained'
+                  href={`/api/v1/deployment/${deployment.uuid}/version/${version.version}/raw/binary`}
+                  target='_blank'
+                  data-test='downloadBinaryFile'
+                >
+                  Download binary file
+                </Button>
+              </Stack>
+            </Box>
+            <Divider orientation='horizontal' />
+          </Box>
+        ))}
+      {versions && versions.length === 0 && <EmptyBlob text='No exportable versions' />}
     </>
   )
 }

diff --git a/types/interfaces.ts b/types/interfaces.ts
@@ -48,8 +48,7 @@ export interface ModelMetadata {
   }
 
   buildOptions?: {
-    exportRawModel: boolean
-    allowGuestDeployments: boolean
+    uploadType: ModelUploadType
   }
 
   // allow other properties