fix: Encode non-finite floats in feature server JSON responses - #6930
Zhuoxi2000 wants to merge 1 commit into
Conversation
A stored NaN or +/-inf float made /get-online-features (and the other endpoints that use convert_response_to_dict) fail with HTTP 500, because JSONResponse serializes with allow_nan=False. Encode non-finite values in float/double scalars, lists and sets as the protobuf JSON strings "NaN", "Infinity" and "-Infinity". Signed-off-by: Edson <zhuoxi2000@gmail.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6930 +/- ##
==========================================
+ Coverage 48.64% 48.66% +0.01%
==========================================
Files 427 427
Lines 53864 53880 +16
Branches 7849 7854 +5
==========================================
+ Hits 26204 26220 +16
Misses 25792 25792
Partials 1868 1868
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
The one failing job, The other four unit-test jobs passed, including |
What this PR does / why we need it:
A stored NaN or +/-inf float value makes
POST /get-online-featuresfail with HTTP 500 (ValueError: Out of range float values are not JSON compliant)._value_to_nativereturns raw Python floats, andJSONResponseserializes withallow_nan=False, so one non-finite value fails the whole multi-feature response. The SDKget_online_features().to_dict()path serves the same value fine./searchand/retrieve-online-documentsshareconvert_response_to_dictand are affected too.This PR encodes non-finite values as the protobuf JSON mapping strings
"NaN","Infinity"and"-Infinity". This applies to:double_val/float_valdouble_list_val,float_list_val,double_set_val,float_set_val(element-wise)Finite values are unchanged, and lists without non-finite values still go through a plain
list(...). Nestedlist_val/map_val/struct_valvalues get the fix through the existing recursion. Theconvert_response_to_dictdocstring documents the encoding.If maintainers would rather emit
nullfor non-finite values (the behaviour whileORJSONResponsewas in use), this is a one-line change in_float_to_native.Not covered here:
RemoteOnlineStore._build_online_response_from_jsoninfers value types from the JSON, so it would read the encoded strings as strings and rejects a list that mixes numbers and strings. This path already failed before (the server returned 500), andnullwould not round-trip through it either. I can follow up on the client side once the encoding is settled.Which issue(s) this PR fixes:
No separate issue; the bug is described above.
Checks
git commit -s)Testing Strategy
New tests:
tests/unit/test_feature_server.py::test_get_online_features_serves_non_finite_float[inf|-inf]: writes +/-inf to the online store, checks that the SDK serves it, then checks that/get-online-featuresreturns 200 with"Infinity"/"-Infinity"and statusPRESENT.tests/unit/test_feature_server_utils.py: covers NaN/inf/-inf fordouble_valandfloat_val, the four float/double list and set types, and ajson.dumps(..., allow_nan=False)round trip ofconvert_response_to_dict.From
sdk/python:assert 500 == 200/Out of range float values are not JSON compliant.tests/unit/test_proto_json.py,tests/unit/test_feature_server_async.pyandtests/unit/infra/feature_servers/test_mcp_server.pyalso pass (28 passed).ruff checkandruff format --check(0.16.9) are clean on the changed files.Misc