Add USE_MINIO feature flag (#477)

* add USE_MINIO feature flag * remove useless django_db marks * refactor feature flag in storage.__init__ logic
codecov · Jan 16, 2025 · ec85262 · ec85262
1 parent 191837f
commit ec85262
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 9 deletions.
diff --git a/shared/rollouts/features.py b/shared/rollouts/features.py
@@ -3,3 +3,4 @@
 BUNDLE_THRESHOLD_FLAG = Feature("bundle_threshold_flag")
 INCLUDE_GITHUB_COMMENT_ACTIONS_BY_OWNER = Feature("include_github_comment_actions")
 USE_NEW_MINIO = Feature("use_new_minio")
+USE_MINIO = Feature("use_minio")
diff --git a/shared/storage/__init__.py b/shared/storage/__init__.py
@@ -1,5 +1,5 @@
 from shared.config import get_config
-from shared.rollouts.features import USE_NEW_MINIO
+from shared.rollouts.features import USE_MINIO, USE_NEW_MINIO
 from shared.storage.aws import AWSStorageService
 from shared.storage.base import BaseStorageService
 from shared.storage.fallback import StorageWithFallbackService
@@ -15,13 +15,12 @@ def get_appropriate_storage_service(
 ) -> BaseStorageService:
     chosen_storage: str = get_config("services", "chosen_storage", default="minio")  # type: ignore
 
-    # TODO: remove this later, as it's temporary for testing the new_minio client
-    if (
-        chosen_storage == "minio"
-        and repoid
-        and USE_NEW_MINIO.check_value(repoid, default=False)
-    ):
-        chosen_storage = "new_minio"
+    if repoid:
+        if USE_MINIO.check_value(repoid, default=False):
+            chosen_storage = "minio"
+
+        if USE_NEW_MINIO.check_value(repoid, default=False):
+            chosen_storage = "new_minio"
 
     if chosen_storage not in _storage_service_cache:
         _storage_service_cache[chosen_storage] = (

diff --git a/tests/unit/storage/test_init.py b/tests/unit/storage/test_init.py
@@ -1,4 +1,4 @@
-from shared.rollouts.features import USE_NEW_MINIO
+from shared.rollouts.features import USE_MINIO, USE_NEW_MINIO
 from shared.storage import get_appropriate_storage_service
 from shared.storage.aws import AWSStorageService
 from shared.storage.fallback import StorageWithFallbackService
@@ -123,6 +123,37 @@ def test_get_appropriate_storage_service_new_minio(
             "minio": minio_config,
         }
         mocker.patch.object(USE_NEW_MINIO, "check_value", return_value=True)
+        mocker.patch.object(USE_MINIO, "check_value", return_value=False)
+        res = get_appropriate_storage_service(repoid=123)
+        assert isinstance(res, NewMinioStorageService)
+        assert res.minio_config == minio_config
+
+    def test_get_appropriate_storage_service_use_minio(
+        self, mock_configuration, mocker
+    ):
+        mock_configuration.params["services"] = {
+            "chosen_storage": "minio",
+            "gcp": gcp_config,
+            "aws": aws_config,
+            "minio": minio_config,
+        }
+        mocker.patch.object(USE_MINIO, "check_value", return_value=True)
+        mocker.patch.object(USE_NEW_MINIO, "check_value", return_value=False)
+        res = get_appropriate_storage_service(repoid=123)
+        assert isinstance(res, MinioStorageService)
+        assert res.minio_config == minio_config
+
+    def test_get_appropriate_storage_service_use_minio_and_new_minio(
+        self, mock_configuration, mocker
+    ):
+        mock_configuration.params["services"] = {
+            "chosen_storage": "minio",
+            "gcp": gcp_config,
+            "aws": aws_config,
+            "minio": minio_config,
+        }
+        mocker.patch.object(USE_MINIO, "check_value", return_value=True)
+        mocker.patch.object(USE_NEW_MINIO, "check_value", return_value=True)
         res = get_appropriate_storage_service(repoid=123)
         assert isinstance(res, NewMinioStorageService)
         assert res.minio_config == minio_config
@@ -137,6 +168,7 @@ def test_get_appropriate_storage_service_new_minio_false(
             "minio": minio_config,
         }
         mocker.patch.object(USE_NEW_MINIO, "check_value", return_value=False)
+        mocker.patch.object(USE_MINIO, "check_value", return_value=False)
         res = get_appropriate_storage_service(repoid=123)
         assert isinstance(res, MinioStorageService)
         assert res.minio_config == minio_config
@@ -152,6 +184,7 @@ def test_get_appropriate_storage_service_new_minio_cached(
         }
 
         mocker.patch.object(USE_NEW_MINIO, "check_value", return_value=False)
+        mocker.patch.object(USE_MINIO, "check_value", return_value=False)
 
         res = get_appropriate_storage_service(repoid=123)