GeezerVison is showing github.com — fix: Encode non-finite floats in feature server JSON responses by Zhuoxi2000 · Pull Request #6930 · feast-dev/feast
fix: Encode non-finite floats in feature server JSON responses
#6930
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from all commits
Commits
File filter
Filter by extension
.py
(3)
All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -6,8 +6,9 @@
import base64
import logging
import math
from datetime import datetime, timezone
from typing import Any, Dict, Optional
from typing import Any, Dict, Optional, Union
from feast.protos.feast.serving.ServingService_pb2 import GetOnlineFeaturesResponse
from feast.protos.feast.types.Value_pb2 import Value
Expand All
@@ -23,18 +24,29 @@
4: "OUTSIDE_MAX_AGE",
}
_FLOAT_SCALAR_FIELDS = ("double_val", "float_val")
_FLOAT_COLLECTION_FIELDS = (
"double_list_val",
"float_list_val",
"double_set_val",
"float_set_val",
)
def convert_response_to_dict(response: GetOnlineFeaturesResponse) -> Dict[str, Any]:
"""Convert GetOnlineFeaturesResponse to a JSON-serializable dict.
Matches the structure produced by MessageToDict(proto, preserving_proto_field_name=True)
with proto_json.patch() applied, with one intentional difference :
with proto_json.patch() applied, with two intentional differences :
- double_val fields are returned as Python float objects (json.dumps uses Python 3.1+
shortest round-trip form, ~15-17 sig digits) rather than 18 fixed significant digits
(float_precision=18). Values are numerically identical; only the JSON string length
may differ. This is safe for all ML feature types and avoids unnecessary precision
overhead.
- Non-finite float values (NaN, +/-inf) are returned as the strings "NaN",
"Infinity" and "-Infinity" (the protobuf JSON mapping), because they are not
valid JSON numbers and would make JSONResponse fail the whole response.
"""
result: Dict[str, Any] = {
"results": [
Expand Down
Expand Up
@@ -77,6 +89,15 @@ def _value_to_native(v: Value) -> Optional[Any]:
which = v.WhichOneof("val")
if which is None or which == "null_val":
return None
# NaN/Infinity are not valid JSON numbers; JSONResponse (allow_nan=False)
# would fail the whole response, so encode them as protobuf JSON does.
elif which in _FLOAT_SCALAR_FIELDS:
return _float_to_native(getattr(v, which))
elif which in _FLOAT_COLLECTION_FIELDS:
vals = list(getattr(v, which).val)
if all(map(math.isfinite, vals)):
return vals
return [_float_to_native(f) for f in vals]
# bytes must be base64-encoded for JSON serialization
Comment thread
Comment on lines
+96
to
101
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more .
When a collection contains a non-finite value, this checks elements until the first such value and then traverses the collection again to build the converted list. For large feature vectors with a late non-finite value, that adds avoidable work. Consider materializing the values once and checking/converting that list.
Suggested:
elif which in _FLOAT_COLLECTION_FIELDS :
vals = getattr (v , which ).val
if all (map (math .isfinite , vals )):
return list (vals )
return [_float_to_native (f ) for f in vals ]
# bytes must be base64-encoded for JSON serialization
vals = list (getattr (v , which ).val )
+ if all (map (math .isfinite , vals )):
+ return vals
+ return [_float_to_native (f ) for f in vals ]
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
elif which == "bytes_val":
return base64.b64encode(v.bytes_val).decode("ascii")
Expand Down
Expand Up
@@ -114,6 +135,19 @@ def _value_to_native(v: Value) -> Optional[Any]:
return getattr(v, which)
def _float_to_native(f: float) -> Union[float, str]:
"""Return a finite float unchanged, or its protobuf JSON string form.
The protobuf JSON mapping encodes non-finite floats as the strings
"NaN", "Infinity" and "-Infinity", which keeps the response valid JSON.
"""
if math.isfinite(f):
return f
if math.isnan(f):
return "NaN"
return "Infinity" if f > 0 else "-Infinity"
def _zoned_timestamp_to_str(zoned) -> Optional[str]:
"""Convert a ZonedTimestamp proto to an ISO 8601 string in its stored zone.
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -782,3 +782,56 @@ def test_get_online_features_returns_feature_view_version_metadata(test_client):
assert [m["name"] for m in metadata["feature_view_metadata"]] == [
"pushed_driver_locations"
]
@pytest.fixture
def store_and_client():
runner = CliRunner()
with runner.local_repo(
get_example_repo("example_feature_repo_1.py"), "file"
) as store:
# raise_server_exceptions=False so the app's own exception handler
# answers the request, exactly as a real client would observe it.
yield store, TestClient(get_app(store), raise_server_exceptions=False)
@pytest.mark.parametrize("non_finite", [float("inf"), float("-inf")])
def test_get_online_features_serves_non_finite_float(store_and_client, non_finite):
"""A stored +/-inf float value must not turn the whole REST response into a 500.
``write_to_online_store`` accepts the value and the SDK path returns it,
so the REST feature server must answer the same request with HTTP 200.
"""
import pandas as pd
store, client = store_and_client
now = _utc_now()
store.write_to_online_store(
"pushed_driver_locations",
pd.DataFrame(
{
"driver_id": [123],
"driver_lat": [non_finite],
"driver_long": ["42.0"],
"event_timestamp": [now],
"created_timestamp": [now],
}
),
)
# Precondition: the Python SDK serves the value without complaint.
sdk_resp = store.get_online_features(
features=["pushed_driver_locations:driver_lat"],
entity_rows=[{"driver_id": 123}],
).to_dict()
assert sdk_resp["driver_lat"] == [non_finite]
response = client.post("/get-online-features", json=get_online_features_body())
assert response.status_code == 200, response.text
body = response.json()
ix = body["metadata"]["feature_names"].index("driver_lat")
assert body["results"][ix]["statuses"] == ["PRESENT"]
assert body["results"][ix]["values"] == [
"Infinity" if non_finite > 0 else "-Infinity"
]
Original file line number
Diff line number
Diff line change
Expand Up
@@ -46,7 +46,9 @@
BoolList,
BytesList,
DoubleList,
DoubleSet,
FloatList,
FloatSet,
Int32List,
Int64List,
Int64Set,
Expand Down
Expand Up
@@ -118,6 +120,36 @@ def test_float_list_val(self):
result = _value_to_native(v)
assert result == [1.5, 2.5]
@pytest.mark.parametrize("field", ["double_val", "float_val"])
@pytest.mark.parametrize(
"value, expected",
[
(float("inf"), "Infinity"),
(float("-inf"), "-Infinity"),
(float("nan"), "NaN"),
],
)
def test_non_finite_scalar_val(self, field, value, expected):
v = Value(**{field: value})
result = _value_to_native(v)
assert result == expected
@pytest.mark.parametrize(
"field, container",
[
("double_list_val", DoubleList),
("float_list_val", FloatList),
("double_set_val", DoubleSet),
("float_set_val", FloatSet),
],
)
def test_non_finite_float_collection_val(self, field, container):
v = Value(
**{field: container(val=[1.5, float("nan"), float("inf"), float("-inf")])}
)
result = _value_to_native(v)
assert result == [1.5, "NaN", "Infinity", "-Infinity"]
def test_int64_list_val(self):
v = Value(int64_list_val=Int64List(val=[100, 200, 300]))
result = _value_to_native(v)
Expand Down
Expand Up
@@ -572,6 +604,26 @@ def test_complex_types_are_json_serializable(self):
serialized = json.dumps(result)
assert serialized # non-empty
def test_non_finite_floats_are_json_serializable(self):
"""NaN/inf must not break strict JSON encoding (JSONResponse uses allow_nan=False)."""
response = GetOnlineFeaturesResponse()
fv = response.results.add()
fv.values.append(Value(double_val=float("inf")))
fv.values.append(Value(float_val=float("-inf")))
fv.values.append(Value(double_list_val=DoubleList(val=[1.0, float("nan")])))
fv.statuses.extend(
[FieldStatus.PRESENT, FieldStatus.PRESENT, FieldStatus.PRESENT]
)
result = convert_response_to_dict(response)
# must not raise
serialized = json.dumps(result, allow_nan=False)
assert json.loads(serialized)["results"][0]["values"] == [
"Infinity",
"-Infinity",
[1.0, "NaN"],
]
class TestProtobufCompatibility:
"""Regression tests for protobuf >= 7.34.0 compatibility (issue #6435).
Expand Down
Loading
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.