-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip version check for cloud v2 rightnow #124
Conversation
src/marqo/client.py
Outdated
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.") | ||
except JSONDecodeError as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what situation do we see a JSONDecodeError
happening in the version check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When it reaches the cloud v2 API and it is not in the skip list. One example is “https://api.marqo.ai/api/”
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): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have one test similar to this but for skipped scenario, where we make sure multiple instantiations make only one get_marqo
attempt and also log a warning?
catch mainline
catch mainline
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
we skip the version check for cloud v2
What is the current behavior? (You can also link to an open issue here)
we have the version check for all urls
What is the new behavior (if this is a feature change)?
we skip version check for specific urls
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
no
Other information:
no