Speed up ci #923
Speed up ci #923
Conversation
improve: Updated vectors with smaller ones
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThis large diff updates the testing framework and configuration across multiple files. A new dependency is added to support parallel testing, and the integration-tests workflow now runs pytest with parallel execution flags and duration reporting. Numerous test files have been modified so that functions no longer rely on a hardcoded collection name; instead, they now accept a dynamically generated collection name (and in some cases, a secondary collection name) as a parameter, which improves test isolation and flexibility. In addition, several tests have reduced the number of vectors processed to speed up execution, and type hints and function signatures have been streamlined for clarity. Minor adjustments to import statements, error handling, and configuration formatting are also included, along with changes to asynchronous tests and migration scripts, all aimed at making the testing suite more modular and adaptable under parallel execution. Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (21)
.github/workflows/integration-tests.yml (1)
72-72: Good performance improvement for fastembed tests.Using parallel execution with
-n autoand duration reporting with--durations=0is a great way to speed up these tests and identify slow performers.Consider also applying the same parallelization to other test commands in this file (like the integration tests on line 62) for consistent performance gains across the entire CI workflow.
tests/congruence_tests/test_nested_filter.py (1)
12-12: Good refactoring for test isolation.Parameterizing the test function with
collection_nameallows for proper test isolation when running in parallel. The consistent use of this parameter across client initialization and API calls ensures that tests won't interfere with each other.For even better clarity, consider also parameterizing the client initialization at lines 15 and 18 to make the dependency on test fixtures explicit throughout the file.
Also applies to: 16-16, 19-19, 50-50, 57-57
tests/test_migrate.py (1)
481-481: Code format improvementThe
strict_mode_confighas been reformatted to a single line for better readability and consistency.tests/congruence_tests/test_has_vector.py (2)
26-27: Duplicate upload calls.The test uploads the same points twice in consecutive calls. This appears to be redundant and could slow down test execution unnecessarily.
- local_client.upload_points(collection_name, points) - remote_client.upload_points(collection_name, points, wait=True)
77-78: Same duplicate upload issue in test_has_vector_multi.Similar to the issue in test_has_vector, there are duplicate consecutive upload calls that appear redundant.
- local_client.upload_points(collection_name, points) - remote_client.upload_points(collection_name, points, wait=True)tests/congruence_tests/test_sparse_idf_search.py (1)
30-32: Consider removing default collection name parameter.While the change to accept a dynamic
collection_nameis good, keepingCOLLECTION_NAMEas a default value contradicts the pattern of removing hardcoded collection names. Consider either removing the default value or defining a local constant instead of importing it.- def simple_search_text( - self, client: QdrantBase, collection_name: str = COLLECTION_NAME - ) -> list[models.ScoredPoint]: + def simple_search_text( + self, client: QdrantBase, collection_name: str + ) -> list[models.ScoredPoint]:tests/congruence_tests/test_collections.py (1)
86-89: Consider using contextlib.suppress for cleaner error handlingBoth try-except-pass blocks could be replaced with
contextlib.suppress()for more concise and idiomatic Python code.- try: - remote_client.delete_collection(collection_name) - except UnexpectedResponse: - pass # collection does not exist + from contextlib import suppress + with suppress(UnexpectedResponse): + remote_client.delete_collection(collection_name) - try: - local_client.delete_collection(collection_name) - except ValueError: - pass # collection does not exist + with suppress(ValueError): + local_client.delete_collection(collection_name)Also applies to: 93-96
🧰 Tools
🪛 Ruff (0.8.2)
86-89: Use
contextlib.suppress(UnexpectedResponse)instead oftry-except-pass(SIM105)
tests/congruence_tests/test_search_distance_matrix.py (1)
18-18: Consider the impact of reducing test data points.Reducing TEST_NUM_POINTS from 100 to 10 will speed up tests but might reduce test coverage. Ensure this change doesn't affect the reliability of your test results.
tests/test_qdrant_client.py (1)
558-558: Inconsistent collection name reference.This line still references the hardcoded
COLLECTION_NAME_ALIASwhile the rest of the file uses the parameterizedcollection_name.Consider updating this to use a dynamic alias name derived from the parameterized collection name:
-test_collection = client.get_collection(COLLECTION_NAME_ALIAS) +test_collection = client.get_collection(f"{collection_name}_alias")And update line 529 similarly:
- collection_name=collection_name, alias_name=COLLECTION_NAME_ALIAS + collection_name=collection_name, alias_name=f"{collection_name}_alias"tests/congruence_tests/test_sparse_search.py (1)
334-334: Remove unnecessary f-string prefixes.These strings don't contain any interpolation placeholders, so the
fprefix is unnecessary.Apply this fix to all four instances:
-local_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) +local_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)Also applies to: 346-346, 384-384, 401-401
🧰 Tools
🪛 Ruff (0.8.2)
334-334: f-string without any placeholders
Remove extraneous
fprefix(F541)
tests/congruence_tests/test_search.py (6)
231-231: Unused loop control variable.The loop control variable
iis not used within the loop body.-for i in range(100): +for _ in range(100):🧰 Tools
🪛 Ruff (0.8.2)
231-231: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
303-303: Unused loop control variable.The loop control variable
iis not used within the loop body.-for i in range(100): +for _ in range(100):🧰 Tools
🪛 Ruff (0.8.2)
303-303: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
358-358: Remove unnecessary f-string prefix.This string doesn't contain any placeholders, so the
fprefix is unnecessary.- local_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + local_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)🧰 Tools
🪛 Ruff (0.8.2)
358-358: f-string without any placeholders
Remove extraneous
fprefix(F541)
366-366: Remove unnecessary f-string prefix.This string doesn't contain any placeholders, so the
fprefix is unnecessary.- remote_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + remote_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)🧰 Tools
🪛 Ruff (0.8.2)
366-366: f-string without any placeholders
Remove extraneous
fprefix(F541)
397-397: Remove unnecessary f-string prefix.This string doesn't contain any placeholders, so the
fprefix is unnecessary.- local_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + local_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)🧰 Tools
🪛 Ruff (0.8.2)
397-397: f-string without any placeholders
Remove extraneous
fprefix(F541)
410-410: Remove unnecessary f-string prefix.This string doesn't contain any placeholders, so the
fprefix is unnecessary.- remote_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + remote_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)🧰 Tools
🪛 Ruff (0.8.2)
410-410: f-string without any placeholders
Remove extraneous
fprefix(F541)
tests/congruence_tests/test_group_search.py (2)
480-480: Remove unnecessary f-string prefix.This string doesn't contain any placeholders, so the
fprefix is unnecessary.- local_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + local_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)🧰 Tools
🪛 Ruff (0.8.2)
480-480: f-string without any placeholders
Remove extraneous
fprefix(F541)
487-487: Remove unnecessary f-string prefix.This string doesn't contain any placeholders, so the
fprefix is unnecessary.- remote_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + remote_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)🧰 Tools
🪛 Ruff (0.8.2)
487-487: f-string without any placeholders
Remove extraneous
fprefix(F541)
tests/congruence_tests/test_discovery.py (1)
63-66: Consider adding a return type annotation for consistency.While the signature has been updated to include
QdrantClientas a return type for other fixtures, it's missing from thegrpc_clientfixture. Consider adding the return type annotation for consistency with other similar methods.-def grpc_client() -> QdrantClient: +def grpc_client() -> QdrantClient:tests/congruence_tests/test_query.py (2)
1547-1547: Remove extraneous f-string prefixes.There are several instances of f-strings without any placeholders that could be simplified to regular strings.
- local_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + local_client.set_payload(collection_name, {"test": "test"}, payload_update_filter) - http_client.set_payload(collection_name, {"test": f"test"}, payload_update_filter) + http_client.set_payload(collection_name, {"test": "test"}, payload_update_filter)Also applies to: 1556-1556, 1588-1588, 1602-1602
🧰 Tools
🪛 Ruff (0.8.2)
1547-1547: f-string without any placeholders
Remove extraneous
fprefix(F541)
1608-1608: Rename unused loop variable for clarity.The loop control variable
iis not used within the loop body. Consider renaming it to_to indicate it's intentionally unused.- for i in range(10): + for _ in range(10):🧰 Tools
🪛 Ruff (0.8.2)
1608-1608: Loop control variable
inot used within loop bodyRename unused
ito_i(B007)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
.github/workflows/integration-tests.yml(1 hunks)pyproject.toml(1 hunks)tests/congruence_tests/conftest.py(1 hunks)tests/congruence_tests/test_aliases.py(4 hunks)tests/congruence_tests/test_collections.py(8 hunks)tests/congruence_tests/test_common.py(2 hunks)tests/congruence_tests/test_count.py(1 hunks)tests/congruence_tests/test_delete_points.py(1 hunks)tests/congruence_tests/test_discovery.py(7 hunks)tests/congruence_tests/test_facet.py(4 hunks)tests/congruence_tests/test_geo_polygon_filter_query.py(2 hunks)tests/congruence_tests/test_group_recommend.py(7 hunks)tests/congruence_tests/test_group_search.py(13 hunks)tests/congruence_tests/test_has_vector.py(3 hunks)tests/congruence_tests/test_multivector_updates.py(8 hunks)tests/congruence_tests/test_nested_filter.py(2 hunks)tests/congruence_tests/test_optional_vectors.py(10 hunks)tests/congruence_tests/test_payload.py(11 hunks)tests/congruence_tests/test_query.py(49 hunks)tests/congruence_tests/test_query_batch.py(1 hunks)tests/congruence_tests/test_recommendation.py(15 hunks)tests/congruence_tests/test_retrieve.py(4 hunks)tests/congruence_tests/test_scroll.py(4 hunks)tests/congruence_tests/test_scroll_order_by.py(3 hunks)tests/congruence_tests/test_search.py(5 hunks)tests/congruence_tests/test_search_distance_matrix.py(5 hunks)tests/congruence_tests/test_sparse_discovery.py(10 hunks)tests/congruence_tests/test_sparse_idf_search.py(5 hunks)tests/congruence_tests/test_sparse_recommend.py(14 hunks)tests/congruence_tests/test_sparse_search.py(5 hunks)tests/congruence_tests/test_sparse_updates.py(8 hunks)tests/congruence_tests/test_unflatten_values_methods.py(4 hunks)tests/congruence_tests/test_updates.py(12 hunks)tests/embed_tests/test_local_inference.py(55 hunks)tests/integration-tests.sh(1 hunks)tests/test_async_qdrant_client.py(18 hunks)tests/test_migrate.py(3 hunks)tests/test_qdrant_client.py(78 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/congruence_tests/test_delete_points.py
51-51: Undefined name COLLECTION_NAME
(F821)
52-52: Undefined name COLLECTION_NAME
(F821)
tests/congruence_tests/test_collections.py
86-89: Use contextlib.suppress(UnexpectedResponse) instead of try-except-pass
(SIM105)
93-96: Use contextlib.suppress(ValueError) instead of try-except-pass
(SIM105)
tests/congruence_tests/test_search.py
231-231: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
303-303: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
358-358: f-string without any placeholders
Remove extraneous f prefix
(F541)
366-366: f-string without any placeholders
Remove extraneous f prefix
(F541)
397-397: f-string without any placeholders
Remove extraneous f prefix
(F541)
410-410: f-string without any placeholders
Remove extraneous f prefix
(F541)
tests/congruence_tests/test_sparse_search.py
334-334: f-string without any placeholders
Remove extraneous f prefix
(F541)
346-346: f-string without any placeholders
Remove extraneous f prefix
(F541)
384-384: f-string without any placeholders
Remove extraneous f prefix
(F541)
401-401: f-string without any placeholders
Remove extraneous f prefix
(F541)
tests/congruence_tests/test_group_search.py
480-480: f-string without any placeholders
Remove extraneous f prefix
(F541)
487-487: f-string without any placeholders
Remove extraneous f prefix
(F541)
tests/congruence_tests/test_query.py
1547-1547: f-string without any placeholders
Remove extraneous f prefix
(F541)
1556-1556: f-string without any placeholders
Remove extraneous f prefix
(F541)
1588-1588: f-string without any placeholders
Remove extraneous f prefix
(F541)
1602-1602: f-string without any placeholders
Remove extraneous f prefix
(F541)
1608-1608: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Redirect rules - poetic-froyo-8baba7
- GitHub Check: Header rules - poetic-froyo-8baba7
- GitHub Check: Pages changed - poetic-froyo-8baba7
- GitHub Check: Python 3.13.x on ubuntu-latest test
- GitHub Check: Python 3.12.x on ubuntu-latest test
- GitHub Check: Python 3.11.x on ubuntu-latest test
- GitHub Check: Python 3.10.x on ubuntu-latest test
- GitHub Check: Python 3.9.x on ubuntu-latest test
- GitHub Check: Python 3.13 test
🔇 Additional comments (133)
tests/congruence_tests/conftest.py (1)
1-1: Good improvements for enabling parallel test execution.These changes significantly improve the test infrastructure by:
- Using dynamically generated collection names with UUIDs
- Making client fixtures session-scoped
- Parameterizing storage and gRPC preferences
This approach ensures test isolation when running in parallel with pytest-xdist, preventing test interference that would otherwise occur with hardcoded collection names.
Also applies to: 12-44
pyproject.toml (1)
38-38: Great addition for CI performance.Adding
pytest-xdistenables parallel test execution, which directly supports the PR's goal of speeding up CI. This package works well with the fixture changes in conftest.py to ensure tests can run concurrently without interference.tests/congruence_tests/test_geo_polygon_filter_query.py (3)
13-13: Good parameterization of the test functionAdding the
collection_nameparameter allows this test to run with different collection names, enabling concurrent execution without collection name collisions. This is a good practice for test isolation.
20-20: Correctly propagating the collection_name parameterThe test now properly passes the collection_name parameter to the init_client calls, ensuring that both local and remote clients are initialized with the same collection name.
Also applies to: 23-23
61-62: Proper usage of the collection_name parameter in API callsThe scroll method calls have been updated to use the collection_name parameter instead of a hardcoded value, ensuring consistency throughout the test function.
Also applies to: 68-69
tests/test_migrate.py (2)
19-19: Good optimization by reducing test vector countReducing
VECTOR_NUMBERfrom 1000 to 100 significantly decreases test execution time while still providing sufficient test coverage. This change directly contributes to speeding up the CI pipeline.
493-493: Syntax fix with trailing commaAdded trailing comma for consistency, which makes future additions to the parameter list cleaner (prevents diff noise).
tests/congruence_tests/test_optional_vectors.py (2)
19-19: Good refactoring to accept dynamic collection namesBoth test functions now accept a
collection_nameparameter, which improves test isolation and allows for parallel test execution without collection name collisions.Also applies to: 91-91
23-26: Consistent use of the collection_name parameterAll client method calls have been consistently updated to use the dynamically provided collection_name parameter instead of a hardcoded constant. This consistency ensures that the tests are properly isolated when run in parallel.
Also applies to: 33-41, 48-49, 67-74, 81-85, 98-110, 117-125, 132-136, 151-158, 165-169
tests/integration-tests.sh (2)
42-47: Excellent CI optimization with parallel test executionThe test execution commands now use pytest's parallel execution capability (
-n auto) which will significantly speed up the test suite by utilizing multiple cores. Additionally, the migration tests are now run in the background, allowing for better concurrency.
49-49: Proper process management with wait commandAdding the
waitcommand ensures that the script won't exit until all background test processes have completed, preventing premature termination of the test run.tests/congruence_tests/test_common.py (2)
27-27: Reduced NUM_VECTORS from 1000 to 100This change reduces the number of test vectors by a factor of 10, which will significantly speed up the test execution time while still providing adequate test coverage.
112-114: Good refactoring to accept a dynamic collection nameMaking this function accept a collection name parameter improves flexibility and follows the pattern of parametrizing collection names across the test suite. This change enables better test isolation and supports parallel test execution.
tests/test_async_qdrant_client.py (3)
4-4: Added uuid module for generating unique collection namesGood addition that supports creating unique collection names for test isolation.
23-24: Type hint added and unique collection name generationThe type hint for prefer_grpc parameter improves code clarity, and generating a unique collection name for each test run enables parallel test execution without conflicts.
567-569: Parametrized test_custom_sharding with collection_nameSimilar to other tests, this function now uses a unique collection name which improves test isolation and enables parallel execution.
tests/congruence_tests/test_aliases.py (3)
19-23: Added collection_name parameter to list_collection_aliases methodGood change to make the method more flexible by accepting a collection name parameter with a default value. This improves reusability and supports the parallel test execution strategy.
26-26: Added collection_name parameter to test_alias_changes functionAdding the collection_name parameter to this test function makes it more configurable and allows it to work with dynamically generated collection names, supporting parallel test execution.
49-54: Properly passing collection_name to comparison functionsThese changes ensure the correct collection name is used when comparing results between local and remote clients, maintaining test consistency when using dynamic collection names.
tests/congruence_tests/test_unflatten_values_methods.py (3)
90-90: Added collection_name parameter to test_values_count_queryAdded collection_name parameter which allows this test to run with a dynamic collection name, facilitating parallel test execution.
153-153: Added collection_name parameter to test_is_empty with proper type annotationGood practice to include the type annotation for the collection_name parameter, making the function signature clearer and enabling better IDE support and static type checking.
208-208: Added collection_name parameter to test_is_nullSimilar to other tests, this function now accepts a collection_name parameter to support dynamic collection names, improving test isolation for parallel execution.
tests/congruence_tests/test_retrieve.py (2)
13-13: Good parameterization refactoring!The changes to accept a dynamic
collection_nameparameter instead of using a hardcoded constant improve test isolation and will enable parallel test execution. The addition of proper collection initialization and standardizing vector count withNUM_VECTORSalso enhances test maintainability.Also applies to: 16-16, 19-21, 25-26
82-83: Function signature successfully updated for parameterization.The test_sparse_retrieve function has been properly updated to accept a dynamic collection name, which aligns with the changes made to other functions in this test suite.
tests/congruence_tests/test_delete_points.py (2)
14-14: Reducing test vectors from 100 to 10 will speed up tests.This change aligns well with the PR objective of speeding up CI, as it reduces the number of test vectors by 90%.
17-18: Well-structured test parameterization.The changes to accept a dynamic
collection_nameparameter and initialize the collections properly will improve test isolation and enable parallel execution.Also applies to: 21-24
tests/congruence_tests/test_has_vector.py (2)
17-28: Good parameterization with proper initialization.The function has been properly updated to accept a dynamic collection name and initialize the collections, which will improve test isolation and enable parallel test execution.
40-48: Consistent parameterization across related functions.Good job updating all related functions to use the collection_name parameter consistently, which ensures the entire test file follows the same pattern.
Also applies to: 65-73
tests/congruence_tests/test_sparse_idf_search.py (1)
97-130: Good refactoring of test_search_with_persistence.The changes to accept
tmp_pathandcollection_nameparameters, along with the simplified temporary directory handling, improve both the test's clarity and flexibility. The updated client initialization and result comparison also align well with the overall refactoring pattern.tests/congruence_tests/test_sparse_updates.py (5)
21-30: Good refactoring of test_upsert to support dynamic collection namesThe addition of the
collection_nameparameter improves test isolation, which is essential for enabling parallel test execution. The function signature update and consistent usage throughout the function body is well-implemented.
99-108: Properly parameterized test_upload_collection functionThe function now correctly accepts and uses the collection_name parameter, making it more flexible and reusable. This change enables tests to run in parallel with unique collection names, preventing conflicts.
129-145: Good implementation of collection name parameterization in generators testThe updates to
test_upload_collection_generatorsproperly implement the collection name parameterization. The consistent use of the parameter in all client calls ensures the test works reliably with dynamic collection names.
162-182: Consistent parameterization applied to test_upload_pointsThe refactoring is consistent with the other functions in this file, properly passing the collection_name parameter to all relevant method calls. This enables parallel test execution without collection name conflicts.
185-217: Well-implemented parameterization in test_upload_uuid_in_batchesThe function correctly accepts and uses the collection_name parameter throughout, maintaining consistency with the other test functions. This change supports the goal of speeding up CI through parallel test execution.
tests/congruence_tests/test_scroll.py (4)
19-40: Good implementation of collection name parameterization in TestSimpleScrollerThe modification to the
scroll_allmethod to accept a dynamic collection name while maintaining backward compatibility with a default value is well-implemented. This approach allows for better test isolation when running tests in parallel.
43-56: Properly updated test_simple_search with collection_name parameterThe function now correctly accepts and uses the collection_name parameter, propagating it to both
init_clientandcompare_client_resultscalls. This ensures proper test isolation.
59-76: Well-implemented parameterization in test_simple_sparse_scrollThe function correctly accepts and uses the collection_name parameter throughout, maintaining consistency with the other test functions. All client initialization and comparison calls properly utilize the parameter.
79-120: Consistent parameterization in mixed IDs testsBoth
test_mixed_idsandtest_sparse_mixed_idshave been consistently updated to support dynamic collection names. The changes are well-implemented across all client initialization and result comparison calls.tests/congruence_tests/test_multivector_updates.py (4)
18-18: Reduced test vector count improves test speedReducing
UPLOAD_NUM_VECTORSfrom 100 to 10 will significantly speed up test execution while still providing adequate coverage. This change directly supports the PR's goal of speeding up CI.
21-94: Well-implemented test_upsert with reduced vector count and collection name parameterizationThe function now correctly uses the smaller vector count and accepts a collection_name parameter, which is consistently used throughout all client methods. These changes will improve test execution speed and enable parallel testing.
97-174: Consistent parameterization applied to collection upload/point testsAll three functions (
test_upload_collection,test_upload_collection_generators, andtest_upload_points) have been consistently updated to support dynamic collection names and use the reduced vector count. These changes enable faster parallel test execution.
177-207: Well-implemented parameterization in test_upload_uuid_in_batches with reduced vector countThe function correctly accepts and uses the collection_name parameter throughout, while also benefiting from the reduced vector count. This change supports both speed improvements and parallel test execution.
tests/congruence_tests/test_count.py (3)
14-28: Good implementation of parameterized count helper functionsBoth
count_allandfilter_countfunctions now accept acollection_nameparameter with sensible defaults, providing flexibility while maintaining backward compatibility. The implementation is clean and properly passes the parameter to the client methods.
30-51: Well-implemented test_simple_count with collection name parameterizationThe function now accepts client instances and a collection_name parameter, using them consistently throughout. The use of underscore as a loop variable is a good practice for indicating unused variables.
53-79: Consistent parameterization in test_simple_sparse_searchThe function has been updated in a similar manner to
test_simple_count, accepting collection_name as a parameter and using it consistently. This consistency across test functions makes the code more maintainable.tests/congruence_tests/test_collections.py (8)
2-2: Properly extended typing imports to include AnyThe import statement now includes
Anyfrom thetypingmodule, which is used to improve type hinting in thewait_forfunction signature.
15-21: LGTM! Improved test isolation with collection_name parameterThe function now accepts a dynamic collection name parameter, enabling better test isolation when tests are run in parallel. The comment on line 16-17 aptly explains the reasoning behind this change.
33-34: Updated init_client calls to use collection_name parameterCorrectly updated the client initialization calls to use the dynamic collection name passed to the function.
58-75: LGTM! Test function correctly updated to use collection_name parameterThe
test_recreate_collectionfunction now accepts a collection name parameter and consistently uses it throughout the function, allowing for better test isolation.
77-101: LGTM! Properly parameterized function with collection_nameThe function has been correctly updated to use the collection_name parameter throughout its implementation.
🧰 Tools
🪛 Ruff (0.8.2)
86-89: Use
contextlib.suppress(UnexpectedResponse)instead oftry-except-pass(SIM105)
93-96: Use
contextlib.suppress(ValueError)instead oftry-except-pass(SIM105)
104-170: Consistently updated test_init_from with collection_name parameterThe function has been properly updated to use the collection_name parameter in all relevant API calls.
173-224: LGTM! Correctly parameterized test_config_variationsThe function has been properly updated to accept and use the collection_name parameter, which enhances test isolation and flexibility.
227-228: Improved type hints in wait_for functionChanged generic
Callableto more specificCallable[..., None]and added proper typing for args and kwargs usingAny. This enhances type safety and makes the function signature more expressive.tests/congruence_tests/test_scroll_order_by.py (5)
14-16: LGTM! Enhanced function flexibility with collection_name parameterThe
scroll_all_with_keyfunction now accepts a collection_name parameter with a default value, making it more flexible while maintaining backward compatibility.
30-30: Updated client.scroll call to use the collection_name parameterProperly updated the method call to use the dynamic collection name.
79-88: LGTM! Helper functions updated to support collection_name parameterAll helper functions (
scroll_all_integers,scroll_all_floats,scroll_all_datetimes) have been updated to accept and forward the collection_name parameter, maintaining consistency throughout the codebase.
91-107: Main test function updated to support parallel testingThe
test_simple_scrollfunction now accepts a collection_name parameter and correctly passes it to all relevant method calls, supporting the PR's goal of improving CI speed through parallel test execution.
112-131: LGTM! Updated client result comparisons to use collection_nameAll calls to
compare_client_resultshave been updated to include the collection_name parameter, ensuring consistent collection naming throughout the test.tests/congruence_tests/test_payload.py (9)
17-17: Added import for initialize_fixture_collectionThe import statement now includes
initialize_fixture_collection, which is used in the updated test functions.
20-20: Improved test performance by reducing vector countThe number of vectors used in tests has been reduced from 100 to 5, which will significantly speed up test execution while still providing sufficient test coverage. This aligns with the PR objective of improving CI speed.
23-28: Enhanced upload function with dynamic collection name and initializationThe function has been improved to:
- Accept dynamic collection_name parameter
- Take explicit client parameters rather than creating them internally
- Support customizing the number of vectors
These changes make the function more reusable and configurable.
31-33: Added proper test fixture initializationNow explicitly initializing fixture collections for both clients before uploading points, which ensures tests start with a clean, consistent state.
39-45: LGTM! Parameterized test_delete_payload with collection_nameThe function now accepts a collection_name parameter and initializes fixture collections, enhancing test isolation for parallel execution.
89-94: LGTM! Updated test_clear_payload with collection_name parameterThe function now correctly accepts and uses a collection_name parameter for improved test isolation.
115-120: LGTM! Properly parameterized test_update_payloadThe function now correctly initializes fixture collections with the provided collection_name.
181-181: LGTM! Updated test_not_jsonable_payload with collection_name parameterThe function now accepts a collection_name parameter for improved test isolation.
271-271: LGTM! Parameterized test_set_payload_with_key with collection_nameThe function now accepts a collection_name parameter for improved test isolation.
tests/congruence_tests/test_facet.py (9)
2-2: Added proper typing import for AnyThe import for
Anyfrom typing has been added, which is used in function signatures throughout the file.
36-39: LGTM! Updated local_client fixture to accept collection_nameThe fixture now accepts and uses the collection_name parameter for client initialization, supporting parallel test execution.
43-75: LGTM! Updated http_client fixture with collection_name parameterThe fixture now accepts and uses the collection_name parameter for client initialization and index creation, consistently applying the same pattern throughout the code.
85-92: LGTM! Updated test_minimal with collection_name parameterThe function now accepts and uses the collection_name parameter in all client methods.
109-119: LGTM! Properly parameterized test_limit with collection_nameThe function and its inner helper function now accept and use the collection_name parameter.
148-156: LGTM! Updated test_exact with collection_name parameterThe function and its inner helper function now accept and use the collection_name parameter.
173-185: LGTM! Updated test_filtered with collection_name parameterThe function has been properly updated to accept and use the collection_name parameter.
214-225: LGTM! Properly parameterized test_exact_filtered with collection_nameThe function and its inner helper function now accept and use the collection_name parameter.
256-256: LGTM! Updated test_other_types_in_local with collection_name parameterThe function now accepts a collection_name parameter, providing consistent behavior with other tests.
tests/embed_tests/test_local_inference.py (7)
1-1: Good addition for generating unique collection names.Adding the UUID module enables unique collection name generation, which is essential for test isolation during parallel test execution.
17-23: Model name updates look good.The updated model names reflect newer versions, which should provide better performance or compatibility with current requirements.
28-31: Excellent fixture for test isolation.This fixture creates unique collection names for each test function, allowing tests to run in parallel without interfering with each other. This is a key improvement for CI performance.
43-57: Good function signature update.Adding the collection_name parameter to populate_dense_collection makes the function more flexible and supports the parallel testing improvements.
59-80: Good function signature update.Adding the collection_name parameter to populate_sparse_collection is consistent with the overall approach to parameterize collection names throughout the test suite.
82-133: Test function update looks good.The test_upsert function now properly accepts and uses the collection_name parameter, ensuring test isolation.
1302-1350: Test parameter updates are correct.The test_upload_mixed_batches_upload_points function correctly uses the collection_name parameter in all appropriate places, maintaining consistency with the rest of the file changes.
tests/congruence_tests/test_group_recommend.py (8)
23-36: Good method signature update.Adding a collection_name parameter with a default value to simple_recommend_groups_image maintains backward compatibility while adding support for dynamic collection naming.
38-52: Consistent parameter handling.The simple_recommend_groups_best_scores method update is consistent with the overall approach to parameterize collection names.
83-103: Well-structured parameter addition.The recommend_groups_from_another_collection method now accepts both primary and secondary collection names with appropriate defaults, enhancing flexibility.
105-121: Good method update.The filter_recommend_groups_text method now properly accepts a collection_name parameter, maintaining consistency with other method updates.
128-141: Test function properly updated.The test_simple_recommend_groups function now accepts collection names and passes them correctly to the init_client calls.
143-175: Parameters properly passed to helper methods.Collection names are correctly passed to the test helper methods, ensuring proper test isolation.
177-203: Consistent parameter passing for all test cases.The collection_name parameter is consistently passed to all test methods throughout the group_by_keys iterations.
207-219: Proper parameter passing for filter tests.The collection_name parameter is correctly passed to the filter_recommend_groups_text method in the filter test loop.
tests/congruence_tests/test_query_batch.py (6)
104-109: Good method signature update.Adding a collection_name parameter with a default value to sparse_query_batch_text maintains backward compatibility while adding support for dynamic collection naming.
111-137: Consistent method updates.All query batch methods (multivec_query_batch_text, dense_query_batch_text, dense_query_batch_image, dense_query_batch_code) are consistently updated to accept the collection_name parameter.
139-144: Static method properly updated.The static dense_query_batch_empty method is correctly updated to accept and use the collection_name parameter.
149-169: Test function properly updated.The test_sparse_query_batch function now accepts a collection_name parameter and correctly passes it to init_client and compare_client_results.
172-188: Consistent parameter handling.The test_multivec_query_batch function uses the collection_name parameter consistently throughout the test.
191-226: Comprehensive test updates.The test_dense_query_batch function correctly passes the collection_name to all relevant method calls, ensuring proper test isolation.
tests/congruence_tests/test_search_distance_matrix.py (7)
27-37: Good fixture updates.The local_client and http_client fixtures now properly accept a collection_name parameter and use it in init_client.
40-43: Fixture return type updated.The grpc_client fixture has been updated to return QdrantClient, maintaining type consistency with other fixtures.
46-55: Type annotations improved.The compare_all_clients_results function now uses QdrantBase instead of QdrantClient, allowing for more flexible client types.
57-79: Test function properly updated.The test_search_offsets_no_filter function and its helper search_offsets_no_filter now properly accept and use the collection_name parameter.
82-104: Consistent parameter handling.The test_search_pairs_no_filter function follows the same pattern as other tests, maintaining consistency.
107-137: Parameter correctly passed to filter tests.The test_search_offsets_filter function properly passes the collection_name to all relevant method calls.
140-170: Consistent approach across all test functions.The test_search_pairs_filter function maintains the same pattern as other tests, ensuring consistent handling of the collection_name parameter.
tests/congruence_tests/test_updates.py (3)
8-8: Good type hint addition for better code clarity.The addition of explicit type hints for
local_clientandremote_clientparameters improves code clarity and helps with IDE autocompletion and static type checking.
22-22: Performance improvement by reducing test vector count.Reducing
UPLOAD_NUM_VECTORSfrom 100 to 10 should significantly speed up test execution while still providing adequate test coverage.
25-27: Good parameterization of collection names.Adding the
collection_nameparameter makes tests more flexible and improves test isolation. This will enable tests to run in parallel without collection name conflicts.tests/test_qdrant_client.py (1)
77-80: Good implementation of dynamic collection naming.This fixture generates unique collection names for each test run, which will prevent test conflicts when running tests in parallel.
tests/congruence_tests/test_sparse_search.py (3)
24-24: Good constant extraction for vector count.Extracting
NUM_VECTORSas a constant makes it easier to adjust the test size in one place.
36-37: Good parameterization of test methods.Adding
collection_nameparameter with a default value makes the methods more flexible while maintaining backward compatibility.Also applies to: 46-48, 57-59
161-161:Details
❓ Verification inconclusive
Unexpected type usage marked by comment.
The comment "why it is not a NamedSparseVector?" suggests potential inconsistency in how query vectors are handled.
This comment indicates potential confusion or inconsistency. Consider investigating whether this method should be using a
NamedSparseVectortype like the other similar methods:
🏁 Script executed:
#!/bin/bash # Check how NamedSparseVector is used in filter_search functions grep -rn "filter_search.*query_vector" --include="*.py" .Length of output: 59
Action Required: Verify consistency for query_vector type usage
The executed grep command did not reveal any instances where
NamedSparseVectoris used in similar contexts (e.g., withinfilter_searchfunctions). This suggests that the use ofself.query_textmight be intentional rather than an oversight. However, the review comment raises a valid question about potential type inconsistencies.Please verify the following:
- Design Intent: Confirm whether the system design mandates that query vectors should be represented as
NamedSparseVectorin all cases, or if using a plain text string (as inself.query_text) is acceptable.- Code Consistency: If other functions or components follow a different pattern (i.e., using
NamedSparseVector), consider standardizing the type across similar methods. Otherwise, update or remove the comment for clarity.tests/congruence_tests/test_sparse_discovery.py (2)
37-42: Good refactoring of client fixtures.Adding explicit type annotations for parameters and supporting collection name parameterization improves test flexibility and type safety.
Also applies to: 61-66, 86-87
97-99: Consistent function signature updates across test methods.All test methods now consistently accept and utilize the
collection_nameparameter, which allows for more flexible and isolated tests.Also applies to: 129-131, 162-164, 188-190, 213-215, 234-236, 256-261, 298-303, 350-352, 371-373
tests/congruence_tests/test_search.py (3)
1-19: LGTM! Proper import organization.The import organization looks good, with necessary imports properly grouped. The numpy import has been moved to the top for better readability.
31-43: Good parameterization of search methods.Adding the
collection_nameparameter with a default value ofCOLLECTION_NAMEmakes the methods more flexible and reusable for different test scenarios. This change supports parallel testing execution.
174-193: Improved test function parameterization.The function now accepts a
collection_nameparameter and correctly passes it to dependent functions, enhancing test isolation when running in parallel.tests/congruence_tests/test_sparse_recommend.py (4)
3-4: LGTM! Proper import organization.The numpy import has been moved to the top of the file for better organization, following Python best practices.
30-46: Well-implemented parameterization for recommendation methods.Adding the
collection_nameparameter with a default value ofCOLLECTION_NAMEimproves the flexibility of these methods for different test scenarios and supports parallel testing.
235-240: Good function signature update for test parameterization.The test function now receives explicit parameters for both collection names, allowing for better test isolation when running in parallel. This is a key improvement for CI speed optimization.
247-275: Correctly updated client initialization with named parameters.The client initialization now properly passes the collection names as named parameters, enhancing code clarity and ensuring the correct collections are used in tests.
tests/congruence_tests/test_recommendation.py (3)
30-46: Well-implemented parameterization in recommendation methods.Adding the
collection_nameparameter with a default value ofCOLLECTION_NAMEenhances the methods' flexibility and supports parallel test execution, a key goal of this PR.
68-86: Good addition of secondary collection parameter.The method now properly handles both primary and secondary collection names, with appropriate defaults, improving the flexibility of cross-collection tests.
235-257: Improved test parameterization for collection references.The test function now properly accepts and passes collection names to dependent functions, supporting parallel test execution for faster CI.
tests/congruence_tests/test_group_search.py (3)
36-54: Well-implemented parameterization for group search methods.Adding the
collection_nameparameter with default values improves flexibility and test isolation for parallel execution.
92-106: Good parameterization for cross-collection operations.The method now accepts both primary and lookup collection names as parameters, with appropriate defaults, enabling more flexible testing scenarios.
252-272: Improved test parameterization with explicit client initialization.The test now properly accepts collection name parameters and passes them to initialization functions, supporting the PR's goal of speeding up CI with parallel test execution.
tests/congruence_tests/test_discovery.py (3)
37-43: Well-structured fixture parameterization.The update to accept
collection_nameandsecondary_collection_nameparameters in the fixture is a good improvement for test isolation, allowing tests to run in parallel with unique collection names.
75-77: Default parameter ensures backward compatibility.Good practice using
COLLECTION_NAMEas the default value for thecollection_nameparameter, ensuring backward compatibility with existing code that might call this function without explicitly providing a collection name.
359-361: Improved test function signature.The test function now accepts collection name parameters which improves test isolation and allows for parallel test execution. This is consistent with the pattern established throughout the file.
tests/congruence_tests/test_query.py (4)
71-73: Consistent method parameterization pattern.The method now accepts
collection_nameparameter with appropriate type annotations, following the pattern established throughout the codebase changes. This enables more flexible test configurations.
820-822: Improved function signature with flexible kwargs.The
init_clientsfunction now accepts arbitrary keyword arguments, making it more flexible for various test scenarios. This is a good improvement that supports the refactoring to use dynamic collection names.
846-848: Enhanced test isolation.Tests now use parameterized collection names rather than hardcoded constants, which improves test isolation and allows for parallel test execution. This is a significant improvement for the CI process.
882-888: Consistent collection name parameterization.The initialization of clients with fixture points now consistently uses the collection name parameters, maintaining the pattern established throughout the file. This is good for code maintainability.
| # await client.upsert(collection_name, points=[models.PointStruct(id=101, vector=np.random.rand(10).tolist())]) | ||
| # assert (await client.get_collection(collection_name)).vectors_count == 101 | ||
| # await client.recover_snapshot(collection_name=collection_name, location=...) | ||
| # assert (await client.get_collection(collection_name)).vectors_count == 100 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Commented out snapshot recovery code
These commented-out sections reference recovery operations with unknown snapshot locations.
Consider either:
- Implementing these tests fully with mock snapshots, or
- Adding a more informative comment explaining why these tests are disabled
🏁 Script executed:
#!/bin/bash
# Check if there are similar commented sections in other test files
rg -A 1 -B 1 "recover.*snapshot" --type py tests/Length of output: 1659
Snapshot Recovery Test Requires Clarification or Mock Implementation
The snapshot recovery tests in tests/test_async_qdrant_client.py remain commented out, with a note indicating that the snapshot location is unknown. Since similar operations appear in tests/type_stub.py, please either:
- Fully implement these tests using mocks to simulate a valid snapshot location,
or - Add a detailed comment explaining why the snapshot recovery tests are disabled (e.g., unavailability of snapshot location or limitations in the test setup).
| # delete non-existent points | ||
| local_client.delete(COLLECTION_NAME, found_ids) | ||
| remote_client.delete(COLLECTION_NAME, found_ids) |
There was a problem hiding this comment.
Fix undefined variable COLLECTION_NAME.
The variable COLLECTION_NAME is no longer imported or defined, which will cause the test to fail. Replace it with the collection_name parameter.
- local_client.delete(COLLECTION_NAME, found_ids)
- remote_client.delete(COLLECTION_NAME, found_ids)
+ local_client.delete(collection_name, found_ids)
+ remote_client.delete(collection_name, found_ids)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # delete non-existent points | |
| local_client.delete(COLLECTION_NAME, found_ids) | |
| remote_client.delete(COLLECTION_NAME, found_ids) | |
| # delete non-existent points | |
| local_client.delete(collection_name, found_ids) | |
| remote_client.delete(collection_name, found_ids) |
🧰 Tools
🪛 Ruff (0.8.2)
51-51: Undefined name COLLECTION_NAME
(F821)
52-52: Undefined name COLLECTION_NAME
(F821)
|
|
||
| @pytest.fixture(scope="module", autouse=True) | ||
| def grpc_client(fixture_points) -> QdrantClient: | ||
| def grpc_client(fixture_points: list[models.PointStruct]) -> QdrantClient: |
There was a problem hiding this comment.
Missing collection_name parameter in grpc_client fixture
While local_client and http_client fixtures have been updated to accept the collection_name parameter, it's missing from the grpc_client fixture signature, which could lead to inconsistent behavior.
-def grpc_client(fixture_points: list[models.PointStruct]) -> QdrantClient:
+def grpc_client(fixture_points: list[models.PointStruct], collection_name: str) -> QdrantClient:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def grpc_client(fixture_points: list[models.PointStruct]) -> QdrantClient: | |
| def grpc_client(fixture_points: list[models.PointStruct], collection_name: str) -> QdrantClient: |
|
closing in favour of #922 |
All Submissions:
devbranch. Did you create your branch fromdev?New Feature Submissions:
pre-commitwithpip3 install pre-commitand set up hooks withpre-commit install?Changes to Core Features: