Initial scaffolding for on demand feature view #1803
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1803 +/- ##
===========================================
- Coverage 84.64% 63.37% -21.28%
===========================================
Files 93 94 +1
Lines 6812 6946 +134
===========================================
- Hits 5766 4402 -1364
- Misses 1046 2544 +1498
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
sdk/python/feast/feature_store.py
Outdated
| for odfv in all_on_demand_feature_views: | ||
| feature_ref = odfv.name | ||
| if feature_ref in _feature_refs: | ||
| ret_value = odfv.udf.__call__(initial_response_df) | ||
| result_row.fields[odfv.name].CopyFrom( | ||
| ValueProto(double_val=ret_value[odfv.features[0].name].values[0]) | ||
| ) |
There was a problem hiding this comment.
Does this mean that we're invoking all the odfvs for every single get_online_features call? Assuming that that's just directional
There was a problem hiding this comment.
Is that not the intended behavior of an "on demand" feature view? We wouldn't materialize this into an online store for example. That'd be what we'd include in a batch feature view.
There's a TODO right above it to only include the right fields from the specified FV, if that's what you're referring to
adchia
changed the title
…for transforms on online fetches Signed-off-by: Danny Chiao <danny@tecton.ai>
Signed-off-by: Danny Chiao <danny@tecton.ai>
Signed-off-by: Danny Chiao <danny@tecton.ai>
Signed-off-by: Danny Chiao <danny@tecton.ai>
Signed-off-by: Danny Chiao <danny@tecton.ai>
| @@ -45,8 +48,9 @@ def test_online_retrieval(environment, universal_data_sources, full_feature_name | |||
| "customer_profile:current_balance", | |||
| "customer_profile:avg_passenger_count", | |||
| "customer_profile:lifetime_trip_count", | |||
| "conv_rate_plus_100", | |||
There was a problem hiding this comment.
Do we handle a case where this odfv name is the same as a feature name, which we try to get using full_feature_name=False?
Should this instead be conv_rate_plus_100:conv_rate_plus_100, which is basically odfvname:featurename?
There was a problem hiding this comment.
@adchia and I talked about this offline; this will likely change in a "prod-ready" release but it's currently experimental.
| @@ -0,0 +1,57 @@ | |||
| // | |||
| // Copyright 2020 The Feast Authors | |||
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, adchia The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
What this PR does / why we need it:
Sets up initial storage and scaffolding for on demand feature view, and supports online fetching. This does not yet support offline feature fetching or request data.
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: