-
Notifications
You must be signed in to change notification settings - Fork 98
Add sharding support #1164
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
Add sharding support #1164
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import pytest | ||
|
|
||
| from tests.common import BASE_URL, REMOTE_MS_1 | ||
| from tests.test_utils import disable_sharding | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("enable_network_options") | ||
| def test_update_and_get_network_settings(client): | ||
| """Test updating and getting network settings.""" | ||
| instance_name = REMOTE_MS_1 | ||
| options = { | ||
| "self": instance_name, | ||
| "remotes": { | ||
| instance_name: { | ||
| "url": BASE_URL, | ||
| "searchApiKey": "search-key-1", | ||
| "writeApiKey": "write-key-1", | ||
| } | ||
| }, | ||
| "sharding": True, | ||
| } | ||
|
|
||
| client.add_or_update_networks(options) | ||
| response = client.get_all_networks() | ||
|
|
||
| assert response["self"] == options["self"] | ||
| assert response["remotes"][instance_name]["url"] == options["remotes"][instance_name]["url"] | ||
| assert ( | ||
| response["remotes"][instance_name]["searchApiKey"] | ||
| == options["remotes"][instance_name]["searchApiKey"] | ||
| ) | ||
| assert ( | ||
| response["remotes"][instance_name]["writeApiKey"] | ||
| == options["remotes"][instance_name]["writeApiKey"] | ||
| ) | ||
| assert response["sharding"] == options["sharding"] | ||
| disable_sharding(client) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,3 +31,8 @@ def test_iso_to_date_time(iso_date, expected): | |
| def test_iso_to_date_time_invalid_format(): | ||
| with pytest.raises(ValueError): | ||
| iso_to_date_time("2023-07-13T23:37:20Z") | ||
|
|
||
|
|
||
| # Refactor to use the unified API to toggle experimental features | ||
| def disable_sharding(client): | ||
| client.add_or_update_networks(body={"sharding": False}) | ||
|
Comment on lines
+37
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add type hints for clarity. The function signature lacks type hints for the Apply this diff to add type hints: -def disable_sharding(client):
+def disable_sharding(client: "meilisearch.Client") -> None:
client.add_or_update_networks(body={"sharding": False})Optionally, add a docstring to document the helper's purpose: def disable_sharding(client: "meilisearch.Client") -> None:
"""Disable sharding experimental feature via the network API."""
client.add_or_update_networks(body={"sharding": False})🤖 Prompt for AI Agents |
||
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.
Clarify or remove the comment.
The comment suggests a future refactor ("Refactor to use the unified API..."), but the code already uses
add_or_update_networks, which appears to be the unified API. Either remove the comment if the refactor is complete, or reword it to accurately describe the function's current purpose.Consider replacing with a descriptive comment:
Or remove the comment entirely if the function name is self-explanatory.
📝 Committable suggestion
🤖 Prompt for AI Agents