Skip to content

fix: Pass native input values to get_online_features from feature server #4117

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

Merged
jeremyary merged 2 commits into from
May 2, 2024
Merged

fix: Pass native input values to get_online_features from feature server #4117

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c87dfda
fix: Pass native input values to get_online_features from feature server
tokoko Apr 19, 2024
3271a5a
remove unnecessary type ignore hint
tokoko Apr 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 8 additions & 18 deletions sdk/python/feast/feature_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@
from fastapi import FastAPI, HTTPException, Request, Response, status
from fastapi.logger import logger
from fastapi.params import Depends
from google.protobuf.json_format import MessageToDict, Parse
from google.protobuf.json_format import MessageToDict
from pydantic import BaseModel

import feast
from feast import proto_json, utils
from feast.constants import DEFAULT_FEATURE_SERVER_REGISTRY_TTL
from feast.data_source import PushMode
from feast.errors import PushSourceNotFoundException
from feast.protos.feast.serving.ServingService_pb2 import GetOnlineFeaturesRequest


# TODO: deprecate this in favor of push features
Expand Down Expand Up @@ -83,34 +82,25 @@ def shutdown_event():
@app.post("/get-online-features")
def get_online_features(body=Depends(get_body)):
try:
# Validate and parse the request data into GetOnlineFeaturesRequest Protobuf object
request_proto = GetOnlineFeaturesRequest()
Parse(body, request_proto)

body = json.loads(body)
Copy link
Member

@franciscojavierarceo franciscojavierarceo Apr 19, 2024

Choose a reason for hiding this comment

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

I'm curious why they serialized this into Protobuf....it was probably on purpose so I'm not exactly sure changes it to Python objects is ideal but if tests pass I suppose it's hard to argue.

Copy link
Collaborator Author

@tokoko tokoko Apr 19, 2024

Choose a reason for hiding this comment

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

I'm not sure about that either. they probably wanted to use it as a form of input validation and to sort of align http and grpc servers.

# Initialize parameters for FeatureStore.get_online_features(...) call
if request_proto.HasField("feature_service"):
if "feature_service" in body:
features = store.get_feature_service(
request_proto.feature_service, allow_cache=True
body["feature_service"], allow_cache=True
)
else:
features = list(request_proto.features.val)

full_feature_names = request_proto.full_feature_names
features = body["features"]

batch_sizes = [len(v.val) for v in request_proto.entities.values()]
num_entities = batch_sizes[0]
if any(batch_size != num_entities for batch_size in batch_sizes):
raise HTTPException(status_code=500, detail="Uneven number of columns")
full_feature_names = body.get("full_feature_names", False)

response_proto = store._get_online_features(
features=features,
entity_values=request_proto.entities,
entity_values=body["entities"],
full_feature_names=full_feature_names,
native_entity_values=False,
).proto

# Convert the Protobuf object to JSON and return it
return MessageToDict( # type: ignore
return MessageToDict(
response_proto, preserving_proto_field_name=True, float_precision=18
)
except Exception as e:
Expand Down