fix: remove naming convention check (#151)

* fix: remove naming convention check, update unit test

* Revert "fix: remove naming convention check, update unit test"

This reverts commit 65d9e2a.

* fix: remove naming convention check, update the docstrings accordingly,update unit tests

* fix: handle empty buckets

* Revert "fix: handle empty buckets"

This reverts commit 42456b8.

* fix: edit a long line

* docs: remove a bullet point on the naming convention check

* fix: rename the docstring and move it up, edit the docstring to clarify what is being tested

* Update tests/test_utils.py

---------

Co-authored-by: Helen Lin <[email protected]>

Author: ajar312

docs/source/UserGuide.rst

@@ -60,8 +60,6 @@ The workflow is generally as follows:
 4. For each prefix, process by checking if it is a new data asset
    and adding it to DocDB if necessary.
 
-   -  If the prefix is not valid (does not adhere to data asset
-      naming convention), log a warning.
    -  If the metadata record exists in S3 but not in DocDB, copy it
       to DocDB.
    -  If the metadata record does not exist in S3, create it and save

pyproject.toml

@@ -59,6 +59,7 @@ exclude = '''
     | _build
     | build
     | dist
+    | .conda
   )/
   | .gitignore
 )
@@ -82,5 +83,5 @@ line_length = 79
 profile = "black"
 
 [tool.interrogate]
-exclude = ["setup.py", "docs", "build"]
+exclude = ["setup.py", "docs", "build", ".conda"]
 fail-under = 100

src/aind_data_asset_indexer/aind_bucket_indexer.py

@@ -37,7 +37,6 @@
     download_json_file_from_s3,
     get_dict_of_core_schema_file_info,
     get_dict_of_file_info,
-    is_prefix_valid,
     is_record_location_valid,
     iterate_through_top_level,
     list_metadata_copies,
@@ -545,15 +544,13 @@ def _process_prefix(
     ):
         """
         Processes a prefix in S3
-        1) If the prefix is not valid (does not adhere to data asset naming
-        convention), log a warning and do not process it further.
-        2) If metadata record exists in S3 and DocDB, do nothing.
-        3) If record is in S3 but not DocDb, then copy it to DocDb if the
+        1) If metadata record exists in S3 and DocDB, do nothing.
+        2) If record is in S3 but not DocDb, then copy it to DocDb if the
         location in the metadata record matches the actual location and
         the record has an _id field. Otherwise, log a warning.
-        4) If record does not exist in both DocDB and S3, build a new metadata
+        3) If record does not exist in both DocDB and S3, build a new metadata
         file and save it to S3 (assume Lambda function will save to DocDB).
-        5) In both cases above, we also copy the original core json files to a
+        4) In both cases above, we also copy the original core json files to a
         subfolder and ensure the top level core jsons are in sync with the
         metadata.nd.json in S3.
 
@@ -569,11 +566,6 @@ def _process_prefix(
 
         """
         bucket = self.job_settings.s3_bucket
-        if not is_prefix_valid(s3_prefix):
-            logging.warning(
-                f"Prefix {s3_prefix} not valid in bucket {bucket}! Skipping."
-            )
-            return
         # Check if metadata record exists
         location = get_s3_location(bucket=bucket, prefix=s3_prefix)
         if location_to_id_map.get(location) is not None:

src/aind_data_asset_indexer/utils.py

@@ -121,8 +121,8 @@ def is_record_location_valid(
       True if there is a location field and the url in the field has a form
       like 's3://{expected_bucket}/prefix'
       Will return False if there is no s3 scheme, the bucket does not match
-      the expected bucket, the prefix contains forward slashes, or the prefix
-      is invalid or does not match the expected prefix.
+      the expected bucket, the prefix contains forward slashes,
+      or does not match the expected prefix.
       If the record name does not match the prefix, a warning is logged,
       but the method will still return True.
 
@@ -143,7 +143,6 @@ def is_record_location_valid(
             if (
                 stripped_prefix == ""
                 or len(stripped_prefix.split("/")) > 1
-                or not is_prefix_valid(stripped_prefix)
                 or (
                     expected_prefix is not None
                     and stripped_prefix != expected_stripped_prefix

tests/test_aind_bucket_indexer.py

@@ -1144,32 +1144,6 @@ def test_process_records(self, mock_dask_bag_map_parts: MagicMock):
         self.basic_job._process_records(example_records)
         mock_dask_bag_map_parts.assert_called()
 
-    @patch("aind_data_asset_indexer.aind_bucket_indexer.does_s3_object_exist")
-    @patch("aind_data_asset_indexer.aind_bucket_indexer.MetadataDbClient")
-    @patch("boto3.client")
-    def test_process_prefix_invalid_prefix(
-        self,
-        mock_s3_client: MagicMock,
-        mock_docdb_client: MagicMock,
-        mock_does_s3_object_exist: MagicMock,
-    ):
-        """Tests _process_prefix method when the prefix is invalid."""
-
-        location_to_id_map = dict()
-        with self.assertLogs(level="DEBUG") as captured:
-            self.basic_job._process_prefix(
-                s3_prefix="ecephys_642478",
-                docdb_client=mock_docdb_client,
-                s3_client=mock_s3_client,
-                location_to_id_map=location_to_id_map,
-            )
-        expected_log_messages = [
-            "WARNING:root:Prefix ecephys_642478 not valid in bucket "
-            "aind-ephys-data-dev-u5u0i5! Skipping."
-        ]
-        self.assertEqual(expected_log_messages, captured.output)
-        mock_does_s3_object_exist.assert_not_called()
-
     @patch(
         "aind_data_asset_indexer.aind_bucket_indexer."
         "upload_metadata_json_str_to_s3"

tests/test_utils.py

@@ -215,6 +215,41 @@ def test_is_record_location_valid_true1(self):
             )
         )
 
+    def test_is_record_location_valid_true2(self):
+        """Tests is_record_location_valid returns true when prefix does not
+        match the naming convention"""
+
+        example_record = {
+            "_id": "abc-123",
+            "name": "prefix1",
+            "location": "s3://some_bucket/prefix1",
+        }
+        self.assertTrue(
+            is_record_location_valid(
+                example_record, expected_bucket="some_bucket"
+            )
+        )
+
+    def test_is_record_location_valid_true3(self):
+        """Tests is_record_location_valid returns true when prefix does
+        not match name"""
+
+        example_record = {
+            "_id": "abc-123",
+            "name": "prefix2_2024-01-01_01-01-01",
+            "location": "s3://some_bucket/prefix1_2024-01-01_01-01-01",
+        }
+        with self.assertLogs(level="DEBUG") as captured:
+            result = is_record_location_valid(
+                example_record, expected_bucket="some_bucket"
+            )
+        expected_log_messages = [
+            "WARNING:root:Record name prefix2_2024-01-01_01-01-01 does not "
+            "match prefix prefix1_2024-01-01_01-01-01."
+        ]
+        self.assertEqual(expected_log_messages, captured.output)
+        self.assertTrue(result)
+
     def test_is_record_location_valid_false0(self):
         """Tests is_record_location_valid returns false when no location field
         is present"""
@@ -275,41 +310,6 @@ def test_is_record_location_valid_false3(self):
         )
 
     def test_is_record_location_valid_false4(self):
-        """Tests is_record_location_valid returns false when prefix is
-        invalid"""
-
-        example_record = {
-            "_id": "abc-123",
-            "name": "prefix1",
-            "location": "s3://some_bucket/prefix1",
-        }
-        self.assertFalse(
-            is_record_location_valid(
-                example_record, expected_bucket="some_bucket"
-            )
-        )
-
-    def test_is_record_location_valid_true2(self):
-        """Tests is_record_location_valid returns true when prefix does
-        not match name"""
-
-        example_record = {
-            "_id": "abc-123",
-            "name": "prefix2_2024-01-01_01-01-01",
-            "location": "s3://some_bucket/prefix1_2024-01-01_01-01-01",
-        }
-        with self.assertLogs(level="DEBUG") as captured:
-            result = is_record_location_valid(
-                example_record, expected_bucket="some_bucket"
-            )
-        expected_log_messages = [
-            "WARNING:root:Record name prefix2_2024-01-01_01-01-01 does not "
-            "match prefix prefix1_2024-01-01_01-01-01."
-        ]
-        self.assertEqual(expected_log_messages, captured.output)
-        self.assertTrue(result)
-
-    def test_is_record_location_valid_false6(self):
         """Tests is_record_location_valid returns false when prefixes don't
         match"""