Skip to content

fix: Disable materialized odfvs for historical retrieval #5880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
astronautas wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Disable materialized odfvs for historical retrieval #5880

astronautas wants to merge 1 commit into from
+24 −0

Conversation

@astronautas
Copy link
Contributor

@astronautas astronautas commented Jan 19, 2026 •
edited
Loading

What this PR does / why we need it:

See here. In essence, makes sure ODFV with write_to_online_store=True - meaning, cached - (on-demand feature views) get actually computed when querying offline store (get_historical_features) for training set generating, since as per docs (and generic expectation), offline stores do not store precomputed on demand feature view historical values (only online stores store their latest values).

Which issue(s) this PR fixes:

#5776

@astronautas astronautas requested a review from a team as a code owner January 19, 2026 07:08
@astronautas
ensure ODFVs are always materialized for offline retrieval
c2e42d0 
Signed-off-by: lukas.valatka <lukas.valatka@cast.ai>
@astronautas astronautas force-pushed the fix/disable-materialized-odfvs-for-historical-retrieval branch from 78ff90f to c2e42d0 Compare January 19, 2026 07:09
@astronautas astronautas changed the title Fix/disable materialized odfvs for historical retrieval fix: disable materialized odfvs for historical retrieval Jan 19, 2026
@astronautas astronautas changed the title fix: disable materialized odfvs for historical retrieval Fix: disable materialized odfvs for historical retrieval Jan 19, 2026
@astronautas astronautas changed the title Fix: disable materialized odfvs for historical retrieval fix: Disable materialized odfvs for historical retrieval Jan 19, 2026
astronautas
astronautas commented Jan 19, 2026
View reviewed changes
pyproject.toml

[dependency-groups]
dev = [
"pytest-xdist>=3.8.0",
Copy link
Contributor Author

@astronautas astronautas Jan 19, 2026

Choose a reason for hiding this comment

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

to ensure parallel test running when needed, handy!

astronautas
astronautas commented Jan 19, 2026
View reviewed changes
Makefile
not test_snowflake" \
sdk/python/tests

test-python-historical-retrieval:
Copy link
Contributor Author

@astronautas astronautas Jan 19, 2026

Choose a reason for hiding this comment

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

was an untested path before, added

astronautas
astronautas commented Jan 19, 2026
View reviewed changes
sdk/python/feast/feature_store.py
# TODO(achal): _group_feature_refs returns the on demand feature views, but it's not passed into the provider.
# This is a weird interface quirk - we should revisit the `get_historical_features` to
# pass in the on demand feature views as well.

Copy link
Contributor Author

@astronautas astronautas Jan 19, 2026

Choose a reason for hiding this comment

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

see the source - only occurs within def get_historical_features(!

@astronautas
Copy link
Contributor Author

astronautas commented Jan 19, 2026

added some explanations for the context

@astronautas
Copy link
Contributor Author

astronautas commented Jan 19, 2026

@franciscojavierarceo

@astronautas
Copy link
Contributor Author

astronautas commented Jan 19, 2026

@ntkathole @franciscojavierarceo

FAILED sdk/python/tests/integration/registration/test_universal_registry.py::test_modify_feature_views_success[gcs_registry] - google.api_core.exceptions.TooManyRequests: 429 PUT https://storage.googleapis.com/upload/storage/v1/b/feast-registry-test-1768823153244/o?uploadType=resumable&upload_id=AJRbA5UkNFZsOvkuNKwLRRw4-oOTH9U9Rue973jdMFgJYYKN8LGRKVCmJ-cUy5ggGbPoW56hTKbEjwLItL1LPOaXkSaVZgZhZQbyu5gL4zsyKA: ***
  "error": ***
    "code": 429,
    "message": "The object feast-registry-test-1768823153244/registry.db exceeded the rate limit for object mutation operations (create, update, and delete). Please reduce your request rate. See https://cloud.google.com/storage/docs/gcs429.",
    "errors": [
      ***
        "message": "The object feast-registry-test-1768823153244/registry.db exceeded the rate limit for object mutation operations (create, update, and delete). Please reduce your request rate. See https://cloud.google.com/storage/docs/gcs429.",
        "domain": "usageLimits",
        "reason": "rateLimitExceeded"
      ***
    ]
  ***

seen this? Seems like a quota.

@astronautas
Copy link
Contributor Author

astronautas commented Jan 19, 2026

@franciscojavierarceo Seems to have passed now 😁

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

Reviewers

@franciscojavierarceo franciscojavierarceo franciscojavierarceo approved these changes

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@astronautas @franciscojavierarceo