fix: Fix five bugs in milvus online store by korbonits · Pull Request #6275 · feast-dev/feast
and others added 3 commits
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>
korbonits
changed the title
fix: fix five bugs in MilvusOnlineStore
fix: Fix five bugs in milvus online store
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Alex Korbonits <alex@korbonits.com>
ntkathole pushed a commit to korbonits/feast that referenced this pull request
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>
ntkathole pushed a commit that referenced this pull request
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters