| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
⚠️ Please install the Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #6859 +/- ##
=======================================
Coverage 47.65% 47.66%
=======================================
Files 422 422
Lines 52396 52396
Branches 7606 7606
=======================================
+ Hits 24970 24973 +3
Misses 25632 25632
+ Partials 1794 1791 -3
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
|
The lint-python failure is coming from master, not from this PR. detect-secrets flags build_deck.py:10 in the Chronon deck added by #6850, where REV is a commit SHA read as a Hex High Entropy String. #6870 fails on the same hook. Happy to send a one-line fix adding a pragma: allowlist secret there, or regenerate .secrets.baseline, whichever you prefer. |
Sorry, something went wrong.
Aggregation.__init__ types time_window and slide_interval as Optional and to_proto honours that: it only writes the Duration when the value is not None. from_proto tests the value instead of the presence, and an absent Duration also reports ToNanoseconds() == 0, so an aggregation defined without a window comes back with timedelta(0). That difference is load bearing. aggregation_specs_to_agg_ops rejects a windowed aggregation in online serving with 'if getattr(agg, "time_window", None) is not None', so an on demand feature view with a plain Aggregation(column=..., function=...) works in process and then fails with 'Time window aggregation is not supported in online serving.' on the first get_online_features after feast apply, whatever the registry. The same round-trip runs for StreamFeatureView. Read the field through HasField, which is available because both are google.protobuf.Duration message fields. Aggregation.__eq__ compares these two attributes, so this also stops a registry diff from seeing an unchanged feature view as modified. Signed-off-by: Rodrigo-Palma <email.rodrigopalma@gmail.com>
| Back | FazBrowse Home | New Git URL |
What this PR does / why we need it
Aggregation.__init__ types time_window and slide_interval as Optional, and to_proto honours that: it only writes the Duration when the value is not None. from_proto tests the value instead of the presence:
An absent Duration also reports ToNanoseconds() == 0, so an aggregation defined without a window comes back as timedelta(0).
That difference is load bearing. aggregation_specs_to_agg_ops rejects a windowed aggregation in online serving with:
So an on demand feature view written the way the existing tests write it:
@on_demand_feature_view(..., aggregations=[Aggregation(column="trips", function="sum")], mode="python")works in process, and then fails on the first get_online_features after feast apply, with "Time window aggregation is not supported in online serving.", even though no window was ever requested. Any registry reaches this, since they all round-trip through the proto. StreamFeatureView runs the same conversion.
Aggregation.__eq__ compares both attributes, so a registry diff also sees an unchanged feature view as modified.
Fix
Read the fields through HasField, which is available because both are google.protobuf.Duration message fields and therefore have presence.
Testing
Two tests in test_on_demand_feature_view_aggregation.py: a plain round-trip that asserts None survives, and an end to end one that takes an ODFV through to_proto/from_proto and then serves it. Both fail before the change and pass after. ruff check, ruff format and mypy are clean on the touched files.