Visitar URL original
fix: Accept the store selector in Faiss configuration by Hanabi9249 · Pull Request #6972 · feast-dev/feast · GitHub
Skip to content

fix: Accept the store selector in Faiss configuration - #6972

Open
Hanabi9249 wants to merge 2 commits into
feast-dev:masterfrom
Hanabi9249:codex/fix-faiss-config-type
Open

Hanabi9249 wants to merge 2 commits into
feast-dev:masterfrom
Hanabi9249:codex/fix-faiss-config-type

Conversation

@Hanabi9249

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Loading the documented Faiss YAML fails because FaissOnlineStoreConfig has no type field and its base model forbids extra fields. RepoConfig forwards the selector in the original dictionary to the configuration model.

Declare the fully qualified store selector with a default. This lets the documented configuration validate and retains the selector when the model is serialized and reconstructed by the online store. Add regression cases for the documented configuration and model round-trip using the existing Faiss test fixture.

Which issue(s) this PR fixes:

No linked issue.

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format.

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

With existing Pydantic 2.12.5 and PyYAML 6.0.3, executed the exact original/fixed configuration class definitions and unchanged FeastConfigBaseModel. The same YAML from the current Faiss documentation reports only type / extra_forbidden before the change and validates afterward. Default selector and model round-trip pass; required-field and unknown-field controls retain their expected rejections. The added regression module compiles, and git diff --cached --check passes.

Validation loaded only source class segments. Full Feast/RepoConfig construction, the repository pytest module (including the added regression methods), CLI, FAISS native import/training/search, full suites and Sphinx were not run. No dependencies were installed.

Release notes

NONE

Signed-off-by: Hanabi <3666353208@qq.com>
@Hanabi9249
Hanabi9249 requested a review from a team as a code owner October 8, 2026 01:02
@codecov-commenter

codecov-commenter commented Oct 9, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.92%. Comparing base (af20fe4) to head (60ecc14).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6972   +/-   ##
=======================================
  Coverage   49.92%   49.92%           
=======================================
  Files         443      443           
  Lines       55511    55512    +1     
  Branches     8096     8096           
=======================================
+ Hits        27714    27715    +1     
  Misses      25874    25874           
  Partials     1923     1923           
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 51.35% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
...on/feast/infra/online_stores/faiss_online_store.py 72.46% <100.00%> (+0.20%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af20fe4...60ecc14. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants