separating the signed ingestion functionality from the original code and adding new logging on crawldir signed usage

Author: bgmello

alephclient/api.py

@@ -472,7 +472,6 @@ def ingest_upload(
         metadata: Optional[Dict] = None,
         sync: bool = False,
         index: bool = True,
-        signed_url: bool = False,
     ) -> Dict:
         """
         Create an empty folder in a collection or upload a document to it
@@ -485,9 +484,6 @@ def ingest_upload(
         files, metadata contains foreign_id of the parent. Metadata for a
         directory contains foreign_id for itself as well as its parent and the
         name of the directory.
-        signed_url: use the signed URL workflow for file uploads. When True,
-        files are uploaded via a signed URL instead of multipart ingest.
-        Directories always use the standard ingest endpoint.
         """
         url_path = "collections/{0}/ingest".format(collection_id)
         params = {"sync": sync, "index": index}
@@ -496,9 +492,6 @@ def ingest_upload(
             data = {"meta": json.dumps(metadata)}
             return self._request("POST", url, data=data)
 
-        if signed_url:
-            return self._signed_url_upload(collection_id, file_path, metadata, index)
-
         for attempt in count(1):
             try:
                 with file_path.open("rb") as fh:
@@ -517,27 +510,51 @@ def ingest_upload(
                 backoff(ae, attempt)
         return {}
 
-    def _signed_url_upload(
+    def signed_url_upload(
         self,
         collection_id: str,
-        file_path: Path,
-        metadata: Optional[Dict],
-        index: bool,
+        file_path: Optional[Path] = None,
+        metadata: Optional[Dict] = None,
+        index: bool = True,
     ) -> Dict:
+        """
+        Upload a document using the signed URL workflow.
+
+        For directories (no file), falls back to the standard ingest endpoint
+        since there is no file content to upload.
+
+        The workflow is:
+        1. POST /file/uploadUrl -> {url, id}
+        2. PUT file content to the signed url
+        3. POST /collections/{id}/document with the upload_id and metadata
+
+        params
+        ------
+        collection_id: id of the collection to upload to
+        file_path: path of the file to upload. None while creating folders
+        metadata: dict containing metadata for the file or folders
+        index: whether to index the document after creation
+        """
+        if not file_path or file_path.is_dir():
+            return self.ingest_upload(
+                collection_id, file_path, metadata=metadata, index=index
+            )
+
         mime_type = mimetypes.guess_type(file_path.name)[0] or MIME
         meta = dict(metadata or {})
         meta["file_name"] = file_path.name
         meta["mime_type"] = mime_type
 
         for attempt in count(1):
             try:
-                # Request a signed upload URL
+                # Step 1: request a signed upload URL
                 upload_url = self._make_url("file/uploadUrl")
                 result = self._request("POST", upload_url)
                 signed_url = result["url"]
                 upload_id = result["id"]
+                log.info("Signed URL [%s]: %s", upload_id, signed_url)
 
-                # PUT file content to the signed URL
+                # Step 2: PUT file content to the signed URL
                 try:
                     with file_path.open("rb") as fh:
                         response = self.session.put(
@@ -549,9 +566,7 @@ def _signed_url_upload(
                 except (RequestException, HTTPError) as exc:
                     raise AlephException(exc) from exc
 
-                # Create document record.
-                # The server returns an empty 200 when a document with
-                # the same foreign_id already exists in the collection.
+                # Step 3: create the document record
                 doc_url_path = f"collections/{collection_id}/document"
                 doc_url = self._make_url(doc_url_path, params={"index": index})
                 payload = {"upload_id": upload_id, "meta": meta}

alephclient/crawldir.py

@@ -130,16 +130,20 @@ def ingest_upload(self, path: Path, parent_id: str, foreign_id: str) -> str:
         log.info("Upload [%s->%s]: %s", self.collection_id, parent_id, foreign_id)
         if parent_id is not None:
             metadata["parent_id"] = parent_id
-        kwargs = {}
         if self.signed_url:
-            kwargs["signed_url"] = True
-        result = self.api.ingest_upload(
-            self.collection_id,
-            path,
-            metadata=metadata,
-            index=self.index,
-            **kwargs,
-        )
+            result = self.api.signed_url_upload(
+                self.collection_id,
+                path,
+                metadata=metadata,
+                index=self.index,
+            )
+        else:
+            result = self.api.ingest_upload(
+                self.collection_id,
+                path,
+                metadata=metadata,
+                index=self.index,
+            )
         if "id" not in result and not hasattr(result, "id"):
             raise AlephException("Upload failed")
         return result["id"]

alephclient/tests/test_tasks.py

@@ -122,7 +122,7 @@ def test_ingest(self, mocker):
             assert call in self.api.ingest_upload.mock_calls
 
     def test_ingest_signed_url(self, mocker):
-        mocker.patch.object(self.api, "ingest_upload", return_value={"id": 42})
+        mocker.patch.object(self.api, "signed_url_upload", return_value={"id": 42})
         mocker.patch.object(
             self.api, "load_collection_by_foreign_id", return_value={"id": 2}
         )
@@ -136,6 +136,4 @@ def test_ingest_signed_url(self, mocker):
             True,
             signed_url=True,
         )
-        assert self.api.ingest_upload.call_count == 6
-        for call in self.api.ingest_upload.call_args_list:
-            assert call.kwargs.get("signed_url") is True
+        assert self.api.signed_url_upload.call_count == 6