fix: Fix five bugs in milvus online store#6275
fix: Fix five bugs in milvus online store#6275franciscojavierarceo merged 7 commits intofeast-dev:masterfrom
Conversation
Add missing RST files for feast.aggregation (including tiling sub-package), feast.dbt, and feast.openlineage. Also add feast.diff.apply_progress which was missing from feast.diff.rst. Update feast.rst toctree to include all three new top-level packages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
No new warning categories are introduced by adding feast.aggregation, feast.dbt, and feast.openlineage to the Sphinx docs. The 10 new "more than one target found" warnings follow the same pre-existing pattern (8 already present) caused by feast.__init__ re-exporting classes under multiple paths. Fixes applied: - Remove :undoc-members: from dataclass automodules (Aggregation, IRMetadata, OpenLineageConfig) to prevent duplicate attribute docs - Document re-exporting __init__ packages at package level only to avoid duplicate descriptions from submodule entries - Fix markdown-style code fences in openlineage/__init__.py docstring - Fix RST formatting in client.py Example section (Example::) and emitter.py ASCII art diagram (literal block) - Fix repo_config.py field docstring that triggered an ambiguous OpenLineageConfig cross-reference Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
1. update() overwrote self._collections with the raw describe_collection() dict from the last table instead of updating the keyed cache entry. _get_or_create_collection() already updates self._collections as a side effect, so the assignment is simply dropped. 2. plan() raised NotImplementedError instead of returning [] like the base class default. This caused `feast plan` to crash for Milvus stores. 3. online_read() carried an extra full_feature_names parameter not present in the OnlineStore base class, violating the interface contract. The parameter was unused and is removed. 4. retrieve_online_documents_v2() passed the raw hit.get() result (which can be None when the composite key is absent) directly to bytes.fromhex(), raising TypeError. Guard added: only call bytes.fromhex() when the value is non-None. 5. Replace print() calls in _connect() with logger.info() so connection messages respect standard logging configuration instead of always writing to stdout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
1. test_milvus_update_preserves_collection_cache — verifies that applying two FeatureViews leaves self._collections as a proper dict keyed by collection name, with a "fields" entry for each view. Previously, update() overwrote _collections with the raw describe_collection() dict from the last table. 2. test_milvus_plan_returns_empty_list — verifies that plan() returns [] instead of raising NotImplementedError, matching the OnlineStore base class default and allowing `feast plan` to work. 3. test_milvus_retrieve_online_documents_v2_missing_entity_key — verifies that a search hit missing the composite primary key produces a None entity_key_proto instead of a TypeError from bytes.fromhex(None). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
|
Hmm, looks like the unit test error for python 3.12 ubuntu was: ________________ tests/unit/local_feast_tests/test_e2e_local.py ________________
[gw6] linux -- Python 3.12.13 /home/runner/work/feast/feast/.venv/bin/python3
worker 'gw6' crashed while running 'tests/unit/local_feast_tests/test_e2e_local.py::test_e2e_local'
=============================== warnings summary ==============================Maybe re-running would resolve? |
There was a problem hiding this comment.
🟡 Falsy check on distance drops valid 0.0 distance values
At line 746, if distance is used to decide whether to wrap the distance in a ValueProto(float_val=...). When distance is 0.0 (a valid distance meaning an exact match for L2 or cosine metrics), Python evaluates 0.0 as falsy, so the code returns an empty ValueProto() instead of ValueProto(float_val=0.0). Because float_val is inside a oneof val in the proto definition (protos/feast/types/Value.proto:78), ValueProto() leaves the oneof unset (WhichOneof("val") → None), while ValueProto(float_val=0.0) correctly sets it to "float_val". This is the same bug pattern the PR fixes for raw_key at line 694. The postgres online store handles this correctly with if distance is not None: (sdk/python/feast/infra/online_stores/postgres_online_store/postgres.py:777).
(Refers to line 746)
Was this helpful? React with 👍 or 👎 to provide feedback.
Adds three cases to test_retrieve_online_milvus_documents that exercise code paths adjacent to recent MilvusOnlineStore bug fixes (feast-dev#6275): 1. Empty-store query before any writes — verifies the v2 retrieval path returns 0 rows instead of raising when the collection exists but is empty. 2. Oversized top_k (top_k=5 on a 3-row dataset) — verifies the hit-parsing loop handles a result set smaller than the requested top_k and returns all available rows. 3. Cosine-metric variant via a second FeatureView with vector_search_metric="COSINE" — verifies the cosine index path end to end (write, index creation, retrieval). No new imports; all types were already imported in the module. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
Adds three cases to test_retrieve_online_milvus_documents that exercise code paths adjacent to recent MilvusOnlineStore bug fixes (#6275): 1. Empty-store query before any writes — verifies the v2 retrieval path returns 0 rows instead of raising when the collection exists but is empty. 2. Oversized top_k (top_k=5 on a 3-row dataset) — verifies the hit-parsing loop handles a result set smaller than the requested top_k and returns all available rows. 3. Cosine-metric variant via a second FeatureView with vector_search_metric="COSINE" — verifies the cosine index path end to end (write, index creation, retrieval). No new imports; all types were already imported in the module. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
# [0.63.0](v0.62.0...v0.63.0) (2026-05-04) ### Bug Fixes * Add project filter to apply_data_source and delete_data_source (closes [#6206](#6206)) ([#6322](#6322)) ([96562c4](96562c4)) * Add project_id filter to SnowflakeRegistry UPDATE path ([#6243](#6243)) ([6658b71](6658b71)), closes [#6208](#6208) [#6208](#6208) * Add subprocess timeouts to prevent test_e2e_local hanging on Dask atexit handler ([3de6556](3de6556)) * Ambiguous truth value of array during materialization ([#6259](#6259)) ([d0c8984](d0c8984)) * Auto-detect GCS/S3 registry store when registry is passed as string ([#6260](#6260)) ([7ebcf03](7ebcf03)) * **bigquery:** Prefer query over table in get_table_query_string ([#6360](#6360)) ([77ed779](77ed779)), closes [#6200](#6200) * correct project_id scoping in get_user_metadata and delete_project ([0c469a7](0c469a7)) * disable Redis RDB persistence in test deployments ([44cd682](44cd682)) * Disable snowflake tests temporarily in CI ([#6356](#6356)) ([31d5a98](31d5a98)) * Filter empty SQL commands at execute_snowflake_statement call sites ([#6249](#6249)) ([92ffbb9](92ffbb9)) * Fix five bugs in milvus online store ([#6275](#6275)) ([212504b](212504b)) * Fix issue with apply feature view ([835cda8](835cda8)) * Fix streaming materialization for exotic sources with lazy UDF pipelines ([c07972d](c07972d)) * Handle missing features gracefully instead of panicking ([7d00b3a](7d00b3a)) * Harden informer cache with label selectors and memory optimizations ([#6242](#6242)) ([3f11356](3f11356)) * **helm:** Avoid nil pointer for metrics.enabled inside podAnnotations ([#6251](#6251)) ([c833f1a](c833f1a)) * Include git in feast server image ([fb03c46](fb03c46)) * Include StreamFeatureView in freshness metric ([#6269](#6269)) ([463f16c](463f16c)) * Pre-create S3A event log dir before SparkContext init ([#6317](#6317)) ([9feca77](9feca77)) * Remote Online Store Type Inference Error with All-NULL Columns ([#6063](#6063)) ([de67bdd](de67bdd)) * Remove selector with kustomize overlay using a JSON 6902 patch ([9107a43](9107a43)) * Resolve multiple bugs in SnowflakeRegistry and Snowflake connection handling ([#6315](#6315)) ([7e66a2e](7e66a2e)) * **spark:** BatchFeatureView with TransformationMode.PYTHON now reads all source columns ([a310eaf](a310eaf)) * **spark:** Use SELECT * when feature_name_columns is empty in pull_all_from_table_or_query ([e1b1d2d](e1b1d2d)) * Support pandas mode in feature builder and fix dask column extraction ([863315e](863315e)) * support SQL string as entity_df in RemoteOfflineStore.get_historical_features ([c559889](c559889)) * Wrap LocalOutputNode return value in ArrowTableValue for consist… ([#6286](#6286)) ([a16cd55](a16cd55)) ### Features * Add agent skills and Cursor/Claude rules for Feast development ([312eea3](312eea3)) * Add feature view versioning support to FAISS online store ([b36acb7](b36acb7)) * Add feature view versioning support to Redis and DynamoDB online stores ([#6257](#6257)) ([edf25af](edf25af)), closes [#6164](#6164) [#6163](#6163) * Add optional 'org' in feature view ([#6288](#6288)) ([#6301](#6301)) ([608b105](608b105)) * Add RaySource, to_ray_dataset first-class method, docs, and tests ([1c98157](1c98157)) * Add TLS support for Go Feature Server ([#6229](#6229)) ([28a58d0](28a58d0)) * Add Vector Search support to MongoDBOnlineStore ([#6344](#6344)) ([c102738](c102738)) * Add versioning support to Milvus online store ([#6330](#6330)) ([3268ced](3268ced)) * Addresses performance issues in the Redis online store ([2e50da0](2e50da0)) * Allow to set gpu for ray ([5580ab4](5580ab4)) * Bump redis-py version cap from <5 to <8 ([#6339](#6339)) ([9538180](9538180)) * Expose feature_server, materialization, and openlineage configuration via FeatureStore CRD ([ec6ecfd](ec6ecfd)) * Make online_write_batch_size configurable in MaterializationConfig ([#6268](#6268)) ([d41becf](d41becf)) * Make udf optional if agg defined ([#5689](#5689)) ([#6328](#6328)) ([f630056](f630056)) * MongoDB offline store ([#6138](#6138)) ([8eebad7](8eebad7)) * Optional input_schema for ODFV ([#6308](#6308)) ([#6312](#6312)) ([f08b4e8](f08b4e8)) * Provision minimal TokenReview RBAC for OIDC auth and add SSL error logging in token parser ([#6240](#6240)) ([dca57e8](dca57e8)) * **spark:** Add compute-on-read support for BatchFeatureView in get_… ([#6357](#6357)) ([630d9f8](630d9f8))
What does this PR do?
Fixes five bugs in
sdk/python/feast/infra/online_stores/milvus_online_store/milvus.pyand adds regression tests for three of them.1.
update()overwrites the collection cache (_collections) instead of updating it_get_or_create_collection()already updatesself._collections[collection_name]as a side effect and returns that entry. The assignment inupdate()replaced the entire_collectionsdict with the rawdescribe_collection()dict for the last table, corrupting all subsequent cache lookups for any other collection.Fix: drop the assignment; the side effect in
_get_or_create_collection()is sufficient.2.
plan()raisesNotImplementedErrorinstead of returning[]The
OnlineStorebase class default isreturn []. RaisingNotImplementedErrorcausedfeast planto crash for any project using the Milvus store.3.
online_read()has an extrafull_feature_namesparameter not in the base classThe parameter was unused in the method body and violates the
OnlineStoreinterface contract (online_readtakesrequested_featuresas the last argument).4.
retrieve_online_documents_v2()crashes withTypeErrorwhen the composite key is absent from a hit5.
_connect()usesprint()instead ofloggingConnection messages now use
logger.info()so they respect the application's logging configuration instead of unconditionally writing to stdout.Tests
Three regression tests added in
tests/unit/online_store/test_online_retrieval.py:test_milvus_update_preserves_collection_cache_collectionscache corruption inupdate()test_milvus_plan_returns_empty_listplan()raisingNotImplementedErrortest_milvus_retrieve_online_documents_v2_missing_entity_keyTypeErrorfrombytes.fromhex(None)Full Milvus unit test results:
ruff checkpasses with no errors.🤖 Generated with Claude Code