Skip to content

fix: Resolve write_to_offline_store feature view with a single registry lookup - #6696

Open
adarshsm wants to merge 1 commit into
feast-dev:masterfrom
adarshsm:fix/write-offline-store-single-lookup
Open

fix: Resolve write_to_offline_store feature view with a single registry lookup#6696
adarshsm wants to merge 1 commit into
feast-dev:masterfrom
adarshsm:fix/write-offline-store-single-lookup

Conversation

@adarshsm

@adarshsm adarshsm commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

What this does

FeatureStore.write_to_offline_store resolved the feature view with a try/except chain (sdk/python/feast/feature_store.py):

try:
    feature_view = self.get_stream_feature_view(...)
except FeatureViewNotFoundException:
    try:
        feature_view = self.get_feature_view(...)
    except FeatureViewNotFoundException:
        feature_view = self.get_label_view(...)

For a plain FeatureView — the common case — the first lookup can never succeed. It is issued, raises FeatureViewNotFoundException, and is discarded, and only then does the lookup that can succeed run. A LabelView pays two failed lookups before the third.

Every getter forwards allow_registry_cache as the registry's allow_cache. On a CachingRegistry the failed attempts come from the cached proto and cost little. On a RemoteRegistry each is a gRPC round-trip to the registry server (it keeps no client-side cache and forwards allow_cache to the server as a request field). This sits on a per-batch write path, so a backfill pays a guaranteed-miss registry RPC before the real one on every batch, and allow_registry_cache=True (the default) does not avoid it.

The fix

Resolve the feature view with a single registry.get_any_feature_view(name, project, allow_cache=...) lookup — the unified accessor added in #4235 for exactly the case where a caller holds only a name. It covers FeatureView, StreamFeatureView, OnDemandFeatureView, and LabelView, so it is a behaviour-preserving replacement for the chain while collapsing up to three registry lookups into one.

#4235 introduced get_any_feature_view to address this same three-getters asymmetry in BaseRegistry and was closed as completed, but this call site was never migrated — it still performed the try/except chain, and a third branch (get_label_view) has been added since.

Testing

Added test_write_to_offline_store_resolves_feature_view_with_single_lookup (sdk/python/tests/unit/test_unit_feature_store.py): it asserts exactly one get_any_feature_view call and that none of the legacy per-type getters fire. The test fails against master (the chain calls get_stream_feature_view, not get_any_feature_view) and passes with this change.

ruff check/ruff format clean on both files; mypy clean on feature_store.py.

Fixes #6671.

…ry lookup

write_to_offline_store resolved the feature view with a try/except chain
that called get_stream_feature_view, then get_feature_view, then
get_label_view in turn. A plain FeatureView -- the common case -- never
matches the first lookup, so it was always issued, failed with
FeatureViewNotFoundException, and discarded before the lookup that could
succeed ran. On a RemoteRegistry each attempt is a gRPC round-trip to the
registry server (no client-side cache), so every batch on this per-batch
write path paid one or more guaranteed-miss RPCs before the real one.
allow_registry_cache=True, the default, does not avoid it on a remote
registry.

Resolve the feature view with a single registry.get_any_feature_view
lookup instead -- the unified accessor added in feast-dev#4235 for exactly the
case where a caller holds only a name. It covers FeatureView,
StreamFeatureView, OnDemandFeatureView, and LabelView, so it is a
behaviour-preserving replacement for the chain while collapsing up to
three registry lookups into one.

Fixes feast-dev#6671.

Signed-off-by: adarshsm <[email protected]>
@adarshsm
adarshsm requested a review from a team as a code owner August 1, 2026 14:03
@codecov-commenter

codecov-commenter commented Aug 1, 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 46.79%. Comparing base (ca355cb) to head (0e63266).
⚠️ Report is 12 commits behind head on master.
❗ 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    #6696      +/-   ##
==========================================
+ Coverage   46.44%   46.79%   +0.34%     
==========================================
  Files         414      414              
  Lines       50134    50185      +51     
  Branches     7173     7181       +8     
==========================================
+ Hits        23285    23483     +198     
+ Misses      25212    25059     -153     
- Partials     1637     1643       +6     
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 48.12% <ø> (+0.37%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/feature_store.py 45.11% <ø> (+0.65%) ⬆️

... and 27 files with indirect coverage changes


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 ca355cb...0e63266. 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.

@franciscojavierarceo franciscojavierarceo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unified get_any_feature_view lookup preserves the supported feature-view types while removing guaranteed-miss RPCs on the remote-registry write path, and the regression asserts the single lookup. I found no blocking issue in the remote diff.

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

Labels

None yet

Projects

None yet

3 participants