Merge pull request #49 from dylanmcreynolds/small_fixes

Multiple fixes for new backend

pyscicat/client.py

@@ -5,7 +5,6 @@
 import hashlib
 import logging
 import json
-import re
 from typing import Optional
 from urllib.parse import urljoin, quote_plus
 
@@ -15,9 +14,9 @@
 
 from pyscicat.model import (
     Attachment,
+    CreateDatasetOrigDatablockDto,
     Dataset,
     Instrument,
-    OrigDatablock,
     Proposal,
     Sample,
 )
@@ -108,25 +107,17 @@ def _call_endpoint(
         endpoint: str,
         data: BaseModel = None,
         operation: str = "",
-        allow_404=False,
     ) -> Optional[dict]:
         response = self._send_to_scicat(cmd=cmd, endpoint=endpoint, data=data)
-        result = response.json()
+
+        result = response.json() if len(response.content) > 0 else None
         if not response.ok:
-            err = result.get("error", {})
-            if (
-                allow_404
-                and response.status_code == 404
-                and re.match(r"Unknown (.+ )?id", err.get("message", ""))
-            ):
-                # The operation failed but because the object does not exist in SciCat.
-                logger.error("Error in operation %s: %s", operation, err)
-                return None
-            raise ScicatCommError(f"Error in operation {operation}: {err}")
+            raise ScicatCommError(f"Error in operation {operation}: {result}")
+
         logger.info(
             "Operation '%s' successful%s",
             operation,
-            f"pid={result['pid']}" if "pid" in result else "",
+            f"pid={result['pid']}" if result and "pid" in result else "",
         )
         return result
 
@@ -200,7 +191,9 @@ def datasets_update(self, dataset: Dataset, pid: str) -> str:
     """
     update_dataset = datasets_update
 
-    def datasets_origdatablock_create(self, origdatablock: OrigDatablock) -> dict:
+    def datasets_origdatablock_create(
+        self, dataset_id: str, datablockDto: CreateDatasetOrigDatablockDto
+    ) -> dict:
         """
         Create a new SciCat Dataset OrigDatablock
         This function has been renamed.
@@ -223,11 +216,11 @@ def datasets_origdatablock_create(self, origdatablock: OrigDatablock) -> dict:
             Raises if a non-20x message is returned
 
         """
-        endpoint = f"Datasets/{quote_plus(origdatablock.datasetId)}/origdatablocks"
+        endpoint = f"Datasets/{quote_plus(dataset_id)}/origdatablocks"
         return self._call_endpoint(
             cmd="post",
             endpoint=endpoint,
-            data=origdatablock,
+            data=datablockDto,
             operation="datasets_origdatablock_create",
         )
 
@@ -518,7 +511,6 @@ def datasets_find(
             cmd="get",
             endpoint=f"Datasets/fullquery?{query}",
             operation="datasets_find",
-            allow_404=True,
         )
 
     """
@@ -559,7 +551,7 @@ def datasets_get_many(self, filter_fields: Optional[dict] = None) -> Optional[di
         filter_fields = json.dumps(filter_fields)
         endpoint = f'Datasets?filter={{"where":{filter_fields}}}'
         return self._call_endpoint(
-            cmd="get", endpoint=endpoint, operation="datasets_get_many", allow_404=True
+            cmd="get", endpoint=endpoint, operation="datasets_get_many"
         )
 
     """
@@ -595,7 +587,6 @@ def published_data_get_many(self, filter=None) -> Optional[dict]:
             cmd="get",
             endpoint=endpoint,
             operation="published_data_get_many",
-            allow_404=True,
         )
 
     """
@@ -620,7 +611,6 @@ def datasets_get_one(self, pid: str) -> Optional[dict]:
             cmd="get",
             endpoint=f"Datasets/{quote_plus(pid)}",
             operation="datasets_get_one",
-            allow_404=True,
         )
 
     get_dataset_by_pid = datasets_get_one
@@ -659,7 +649,6 @@ def instruments_get_one(self, pid: str = None, name: str = None) -> Optional[dic
             cmd="get",
             endpoint=endpoint,
             operation="instruments_get_one",
-            allow_404=True,
         )
 
     get_instrument = instruments_get_one
@@ -685,7 +674,6 @@ def samples_get_one(self, pid: str) -> Optional[dict]:
             cmd="get",
             endpoint=f"Samples/{quote_plus(pid)}",
             operation="samples_get_one",
-            allow_404=True,
         )
 
     get_sample = samples_get_one
@@ -707,7 +695,7 @@ def proposals_get_one(self, pid: str = None) -> Optional[dict]:
             The proposal with the requested pid
         """
         return self._call_endpoint(
-            cmd="get", endpoint=f"Proposals/{quote_plus(pid)}", allow_404=True
+            cmd="get", endpoint=f"Proposals/{quote_plus(pid)}"
         )
 
     get_proposal = proposals_get_one
@@ -732,7 +720,6 @@ def datasets_origdatablocks_get_one(self, pid: str) -> Optional[dict]:
             cmd="get",
             endpoint=f"Datasets/{quote_plus(pid)}/origdatablocks",
             operation="datasets_origdatablocks_get_one",
-            allow_404=True,
         )
 
     get_dataset_origdatablocks = datasets_origdatablocks_get_one
@@ -757,7 +744,6 @@ def datasets_delete(self, pid: str) -> Optional[dict]:
             cmd="delete",
             endpoint=f"Datasets/{quote_plus(pid)}",
             operation="datasets_delete",
-            allow_404=True,
         )
 
     delete_dataset = datasets_delete
@@ -823,9 +809,8 @@ def _log_in_via_auth_msad(base_url, username, password):
         verify=True,
     )
     if not response.ok:
-        err = response.json()["error"]
         logger.error(
-            f'Error retrieving token for user: {err["name"]}, {err["statusCode"]}: {err["message"]}'
+            f'Error retrieving token for user: {response.json()}'
         )
         raise ScicatLoginError(response.content)
 
@@ -846,8 +831,7 @@ def get_token(base_url, username, password):
     if response.ok:
         return response.json()["access_token"]
 
-    err = response.json()["error"]
     logger.error(
-        f' Failed log in:  {err["name"]}, {err["statusCode"]}: {err["message"]}'
+        f' Failed log in:  {response.json()}'
     )
     raise ScicatLoginError(response.content)

pyscicat/model.py

@@ -90,7 +90,9 @@ class Job(MongoQueryable):
     executionTime: Optional[str] = None
     jobParams: Optional[dict] = None
     jobStatusMessage: Optional[str] = None
-    datasetList: Optional[dict] = None  # documentation says dict, but should maybe be list?
+    datasetList: Optional[
+        dict
+    ] = None  # documentation says dict, but should maybe be list?
     jobResultObject: Optional[dict] = None  # ibid.
 
 
@@ -115,7 +117,9 @@ class Dataset(Ownable):
     creationTime: str  # datetime
     datasetName: Optional[str] = None
     description: Optional[str] = None
-    history: Optional[List[dict]] = None  # list of foreigh key ids to the Messages table
+    history: Optional[
+        List[dict]
+    ] = None  # list of foreigh key ids to the Messages table
     instrumentId: Optional[str] = None
     isPublished: Optional[bool] = False
     keywords: Optional[List[str]] = None
@@ -187,25 +191,31 @@ class Datablock(Ownable):
 
     id: Optional[str] = None
     # archiveId: str = None  listed in catamel model, but comes back invalid?
-    size: int
+
     packedSize: Optional[int] = None
     chkAlg: Optional[int] = None
     version: str = None
     instrumentGroup: Optional[str] = None
-    dataFileList: List[DataFile]
     datasetId: str
 
 
-class OrigDatablock(Ownable):
+class CreateDatasetOrigDatablockDto(BaseModel):
+    """
+    DTO for creating a new dataset with an original datablock
+    """
+
+    chkAlg: Optional[int] = None
+    dataFileList: List[DataFile]
+    size: int
+
+
+class OrigDatablock(Ownable, CreateDatasetOrigDatablockDto):
     """
     An Original Datablock maps between a Dataset and contains DataFiles
     """
 
     id: Optional[str] = None
-    # archiveId: str = None  listed in catamel model, but comes back invalid?
-    size: int
     instrumentGroup: Optional[str] = None
-    dataFileList: List[DataFile]
     datasetId: str
 
 

tests/test_pyscicat/test_client.py

@@ -14,7 +14,7 @@
 
 from pyscicat.model import (
     Attachment,
-    Datablock,
+    CreateDatasetOrigDatablockDto,
     DataFile,
     Instrument,
     Proposal,
@@ -138,14 +138,13 @@ def test_scicat_ingest():
 
         # Datablock with DataFiles
         data_file = DataFile(path="/foo/bar", size=42)
-        data_block = Datablock(
+        data_block_dto = CreateDatasetOrigDatablockDto(
             size=42,
-            version="1",
             datasetId=dataset_id,
             dataFileList=[data_file],
-            **ownable.dict()
+
         )
-        scicat.upload_dataset_origdatablock(data_block)
+        scicat.upload_dataset_origdatablock(dataset_id, data_block_dto)
 
         # Attachment
         attachment = Attachment(
@@ -187,17 +186,9 @@ def test_get_dataset():
 def test_get_nonexistent_dataset():
     with requests_mock.Mocker() as mock_request:
         mock_request.get(
-            local_url + "Datasets/74",
-            status_code=404,
-            reason="Not Found",
-            json={
-                "error": {
-                    "statusCode": 404,
-                    "name": "Error",
-                    "message": 'Unknown "Dataset" id "74".',
-                    "code": "MODEL_NOT_FOUND",
-                }
-            },
+            local_url + "datasets/74",
+            status_code=200,
+            content=b''
         )
         client = from_token(base_url=local_url, token="a_token")
         assert client.datasets_get_one("74") is None
@@ -206,7 +197,7 @@ def test_get_nonexistent_dataset():
 def test_get_dataset_bad_url():
     with requests_mock.Mocker() as mock_request:
         mock_request.get(
-            "http://localhost:3000/api/v100/Datasets/53",
+            "http://localhost:3000/api/v100/datasets/53",
             status_code=404,
             reason="Not Found",
             json={