Skip to content

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

Merged
ntkathole merged 1 commit into
feast-dev:masterfrom
adarshsm:fix/write-offline-store-single-lookup
Aug 18, 2026
Merged

fix: Resolve write_to_offline_store feature view with a single registry lookup#6696
ntkathole merged 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.

@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 47.00%. Comparing base (602d752) to head (15888cd).
⚠️ Report is 7 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.91%   47.00%   +0.08%     
==========================================
  Files         416      416              
  Lines       50545    50600      +55     
  Branches     7252     7262      +10     
==========================================
+ Hits        23714    23783      +69     
+ Misses      25157    25129      -28     
- Partials     1674     1688      +14     
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 48.33% <100.00%> (+0.09%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/feature_store.py 43.64% <100.00%> (+0.67%) ⬆️

... and 9 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 3425783...15888cd. 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.

@ntkathole
ntkathole force-pushed the fix/write-offline-store-single-lookup branch from 0e63266 to f148010 Compare August 18, 2026 04:28
…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 <24850536+adarshsm@users.noreply.github.com>
@ntkathole
ntkathole force-pushed the fix/write-offline-store-single-lookup branch from f148010 to 15888cd Compare August 18, 2026 05:03
@ntkathole
ntkathole merged commit a42dc85 into feast-dev:master Aug 18, 2026
9 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants