Skip version check for cloud v2 rightnow (#124)

skip the version check for cloud v2

src/marqo/client.py

@@ -2,6 +2,7 @@
 from typing import Any, Dict, List, Optional, Union
 
 from pydantic import error_wrappers
+from requests.exceptions import RequestException
 
 from marqo.index import Index
 from marqo.config import Config
@@ -11,8 +12,10 @@
 from marqo import errors
 from marqo.version import minimum_supported_marqo_version
 from marqo.marqo_logging import mq_logger
+from marqo.errors import MarqoWebError
 # we want to avoid name conflicts with marqo.version
 from packaging import version as versioning_helpers
+from json import JSONDecodeError
 
 # A dictionary to cache the marqo url and version for compatibility check
 marqo_url_and_version_cache = {}
@@ -199,12 +202,37 @@ def get_cpu_info(self):
         return self.http.get(path="device/cpu")
 
     def _marqo_minimum_supported_version_check(self):
-        if self.url not in marqo_url_and_version_cache:
-            marqo_url_and_version_cache[self.url] = self.get_marqo()["version"]
-        marqo_version = marqo_url_and_version_cache[self.url]
-        if versioning_helpers.parse(marqo_version) < versioning_helpers.parse(minimum_supported_marqo_version()):
-            mq_logger.warning(f"Your Marqo Python client requires a minimum Marqo version of "
-                              f"{minimum_supported_marqo_version()} to function properly, while your Marqo version is {marqo_version}. "
-                              f"Please upgrade your Marqo instance to avoid potential errors. "
-                              f"If you have already upgraded your Marqo instance but still having this warning, please import you Marqo Python client again and retry.")
+        min_ver = minimum_supported_marqo_version()
+        skip_warning_message = (
+            f"Marqo encountered a problem trying to check the Marqo version found at `{self.url}`. "
+            f"The minimum supported Marqo version for this client is {min_ver}. "
+            f"If you are sure your Marqo version is compatible with this client, you can ignore this message. ")
+
+        # Skip the check if the url is previously labelled as "_skipped"
+        if self.url in marqo_url_and_version_cache and marqo_url_and_version_cache[self.url] == "_skipped":
+            mq_logger.warning(skip_warning_message)
+            return
+
+        # Skip the check for Marqo CloudV2 APIs right now
+        skip_version_check_url = ["https://api.marqo.ai", "https://cloud.marqo.ai"]
+        if self.url in skip_version_check_url:
+            marqo_url_and_version_cache[self.url] = "_skipped"
+            mq_logger.warning(skip_warning_message)
+            return
+
+        # Do version check
+        try:
+            if self.url not in marqo_url_and_version_cache:
+                marqo_url_and_version_cache[self.url] = self.get_marqo()["version"]
+            marqo_version = marqo_url_and_version_cache[self.url]
+            if versioning_helpers.parse(marqo_version) < versioning_helpers.parse(min_ver):
+                mq_logger.warning(f"Your Marqo Python client requires a minimum Marqo version of "
+                                  f"{minimum_supported_marqo_version()} to function properly, but your Marqo version is {marqo_version}. "
+                                  f"Please upgrade your Marqo instance to avoid potential errors. "
+                                  f"If you have already changed your Marqo instance but still get this warning, please restart your Marqo client Python interpreter.")
+        except (MarqoWebError, RequestException, TypeError, KeyError) as e:
+            mq_logger.warning(skip_warning_message)
+            marqo_url_and_version_cache[self.url] = "_skipped"
+        return
+
 

tests/v0_tests/test_client.py

@@ -1,8 +1,12 @@
+import requests
+from json import JSONDecodeError
 from unittest import mock
+
 from marqo.client import Client
 from tests.marqo_test import MarqoTestCase
 from marqo.errors import MarqoApiError
 from marqo.client import marqo_url_and_version_cache
+from marqo.errors import BackendTimeoutError, BackendCommunicationError
 
 
 class TestClient(MarqoTestCase):
@@ -33,9 +37,8 @@ def test_health(self):
         assert 'status' in res['backend']
 
     def test_version_check_instantiation(self):
-        with mock.patch("marqo.client.mq_logger.warning") as mock_warning,\
-            mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
-
+        with mock.patch("marqo.client.mq_logger.warning") as mock_warning, \
+                mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
             mock_get_marqo.return_value = {'version': '0.0.0'}
             client = Client(**self.client_settings)
 
@@ -48,26 +51,89 @@ def test_version_check_instantiation(self):
             warning_message = mock_warning.call_args[0][0]
 
             # Assert the message is what you expect
-            self.assertIn("Please upgrade your Marqo instance to avoid potential errors.",  warning_message)
+            self.assertIn("Please upgrade your Marqo instance to avoid potential errors.", warning_message)
 
             # Assert the url is in the cache
             self.assertIn(self.client_settings['url'], marqo_url_and_version_cache)
             assert marqo_url_and_version_cache[self.client_settings['url']] == '0.0.0'
 
+    def test_skip_version_check_for_cloud_v2(self):
+        for url in ["https://api.marqo.ai", "https://cloud.marqo.ai"]:
+            with mock.patch("marqo.client.mq_logger.warning") as mock_warning, \
+                    mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
+                mock_get_marqo.return_value = {'version': '0.0.0'}
+                client = Client(url=url)
+
+                mock_get_marqo.assert_not_called()
+
+                assert url in marqo_url_and_version_cache
+                assert marqo_url_and_version_cache[url] == '_skipped'
+
+    def test_skip_version_check_for_previously_labelled_url(self):
+        with mock.patch.dict("marqo.client.marqo_url_and_version_cache",
+                             {self.client_settings["url"]: "_skipped"}) as mock_cache, \
+                mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
+            client = Client(**self.client_settings)
+
+            mock_get_marqo.assert_not_called()
+
+    def test_error_handling_in_version_check(self):
+        side_effect_list = [requests.exceptions.JSONDecodeError("test", "test", 1), BackendCommunicationError("test"),
+                            BackendTimeoutError("test"), requests.exceptions.RequestException("test"),
+                            KeyError("test"), KeyError("test"), requests.exceptions.Timeout("test")]
+        for i, side_effect in enumerate(side_effect_list):
+            with mock.patch("marqo.client.mq_logger.warning") as mock_warning, \
+                    mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
+                mock_get_marqo.side_effect = side_effect
+                client = Client(**self.client_settings)
+
+                mock_get_marqo.assert_called_once()
+
+                # Check the warning was logged
+                mock_warning.assert_called_once()
+
+                # Get the warning message
+                warning_message = mock_warning.call_args[0][0]
+
+                # Assert the message is what you expect
+                self.assertIn("Marqo encountered a problem trying to check the Marqo version found", warning_message)
+                self.assertEqual(marqo_url_and_version_cache, dict({self.client_settings["url"]: "_skipped"}))
+
+                marqo_url_and_version_cache.clear()
+
     def test_version_check_multiple_instantiation(self):
         """Ensure that duplicated instantiation of the client does not result in multiple APIs calls of get_marqo()"""
         with mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
             mock_get_marqo.return_value = {'version': '0.0.0'}
             client = Client(**self.client_settings)
 
-        mock_get_marqo.assert_called_once()
-        mock_get_marqo.reset_mock()
+            mock_get_marqo.assert_called_once()
+            mock_get_marqo.reset_mock()
 
         for _ in range(10):
             with mock.patch("marqo.client.mq_logger.warning") as mock_warning, \
-                 mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
-
+                    mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
                 client = Client(**self.client_settings)
 
                 mock_get_marqo.assert_not_called()
-                mock_warning.assert_called_once()
+                mock_warning.assert_called_once()
+
+    def test_skipped_version_check_multiple_instantiation(self):
+        """Ensure that the url labelled as `_skipped` only call get_marqo() once"""
+        url = "https://dummy/url"
+        with mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
+            mock_get_marqo.side_effect = requests.exceptions.RequestException("test")
+            client = Client(url=url)
+
+            mock_get_marqo.assert_called_once()
+            mock_get_marqo.reset_mock()
+            assert marqo_url_and_version_cache[url] == '_skipped'
+
+        for _ in range(10):
+            with mock.patch("marqo.client.mq_logger.warning") as mock_warning, \
+                    mock.patch("marqo.client.Client.get_marqo") as mock_get_marqo:
+                client = Client(url=url)
+
+                mock_get_marqo.assert_not_called()
+                # Check the warning was logged every instantiation
+                mock_warning.assert_called_once()