Skip to content

feat: Use ORJSONResponse for faster JSON serialization in feature server#5930

Open
ntkathole wants to merge 1 commit intofeast-dev:masterfrom
ntkathole:orjson
Open

feat: Use ORJSONResponse for faster JSON serialization in feature server#5930
ntkathole wants to merge 1 commit intofeast-dev:masterfrom
ntkathole:orjson

Conversation

@ntkathole
Copy link
Member

@ntkathole ntkathole commented Feb 1, 2026

What this PR does / why we need it:

This PR improves the latency of the online feature server by using FastAPI's built-in ORJSONResponse instead of the default JSONResponse. This leverages orjson, a fast JSON library written in Rust, for response serialization.

Changes

  • Added orjson>=3.9.0 as a core dependency (required by ORJSONResponse)
  • Modified /get-online-features and /retrieve-online-documents endpoint to return ORJSONResponse

When returning a dict from a FastAPI endpoint, FastAPI uses Python's standard json module for serialization. By using ORJSONResponse, we leverage orjson which is written in Rust and provides significantly faster JSON encoding.


Open with Devin

@ntkathole ntkathole self-assigned this Feb 1, 2026
@ntkathole ntkathole requested a review from a team as a code owner February 1, 2026 15:48
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional flags.

Open in Devin Review

Copy link
Member

@franciscojavierarceo franciscojavierarceo left a comment

Choose a reason for hiding this comment

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

Need to get tests passing but approving to unblock

@ntkathole
Copy link
Member Author

Need to get tests passing but approving to unblock

@franciscojavierarceo Test failures are due to transient dependency sqlglot, related to ibis-project/ibis#11882.

I have handled it already in #5928 , will rebase this PR once it's merged.

Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View issue and 4 additional flags in Devin Review.

Open in Devin Review

float_precision=18,
)
return response_dict
return ORJSONResponse(content=response_dict)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 ORJSONResponse serializes NaN/Infinity as null, changing API behavior

Switching from JSONResponse to ORJSONResponse changes how NaN and Infinity float values are serialized in the API response.

Click to expand

Behavior Change

The standard json module (used by JSONResponse) serializes special float values as:

  • float('nan')NaN
  • float('inf')Infinity
  • float('-inf')-Infinity

While orjson (used by ORJSONResponse) serializes them as:

  • float('nan')null
  • float('inf')null
  • float('-inf')null

Impact

The Feast codebase explicitly uses NaN values for missing data. In sdk/python/feast/proto_json.py:99-101:

# Convert each null as NaN.
message.double_list_val.val.extend(
    [item if item is not None else float("nan") for item in value]
)

And tests in sdk/python/tests/unit/test_proto_json.py:65 verify NaN is preserved:

assertpy.assert_that(feature_vector_json["values"][5][3]).is_nan()

With ORJSONResponse, clients that previously received NaN in responses will now receive null, which has different semantics (missing value vs. not-a-number). This is a breaking change for API consumers that rely on distinguishing between null and NaN values.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Member Author

Choose a reason for hiding this comment

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

@franciscojavierarceo Thoughts on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants