feat: Add api contract to fastapi docs #4721

robhowley · 2024-10-30T18:52:01Z

What this PR does / why we need it:

FastApi by default creates an openapi docs page which lists out endpoints
the python feature server doesn't use pydantic models for request bodies, so the sample inputs for the endpoints aren't populated and schema unknown
remove json request body parsing and replace it w pydantic models
this mr does not define a robust response schema for /get-online-features
- simply specifies it as dict[str, any]
- should come up w something better but as a follow up

feast serve --port 8080

generates the following at http://localhost:8080/docs

Which issue(s) this PR fixes:

Fixes #4720

Misc

Signed-off-by: Rob Howley <howley.robert@gmail.com>

robhowley · 2024-10-30T21:58:19Z

sdk/python/feast/feature_server.py

-        body = json.loads(body)
-        full_feature_names = body.get("full_feature_names", False)
-        entity_rows = body["entities"]
+    async def get_online_features(request: GetOnlineFeaturesRequest) -> Dict[str, Any]:


fastapi is smart about types. surfaces pydantic model in the autogenerated api docs. this also gives us schema validation on requests for free. for example

robhowley · 2024-10-30T22:03:10Z

sdk/python/feast/feature_server.py

+    feature_service: Optional[str] = None
+    features: Optional[List[str]] = None


could (should) add validation on the model that throws if both are set. didnt do it here, bc it would be technically breaking. the code works just picks the feature_service value and ignore features. this is an unintuitive (successful) result, seems like it would make more sense to return 422 and say, "gotta pick one". maybe flag it for 1.0? @franciscojavierarceo

franciscojavierarceo

Looks like we're ignoring a lot of types now. Were the type checks nontrivial to add? I've had some challenges in the past so wanted to confirm.

robhowley · 2024-10-31T14:08:26Z

im honestly unsure how this passed before. i think bc it was pulling from json it was just marking everything as dict[str, Any]? it gets an inconsistent assignment type error which makes sense since it's assigning two diff things. i can try and add asserts to please the mypy gods, but didnt want to introduce the risk of throwing where one didnt exist before (even though we know it shouldn't matter) just bc of mypy. i'll defer to you @franciscojavierarceo on this: try and add asserts or let it be.

franciscojavierarceo · 2024-10-31T16:14:13Z

We can add asserts and type check in subsequent PR. Thank you for this!

robhowley · 2024-10-31T16:40:42Z

flake?

dmartinol · 2024-10-31T16:40:47Z

sdk/python/feast/feature_server.py

-        for feature_view in request.feature_views:
+    def materialize(request: MaterializeRequest) -> None:
+        for feature_view in request.feature_views or []:
+            # TODO: receives a str for resource but isn't in the Union. is str actually allowed?


it's not allowed, so we should fetch the feature_view instance as we do in the write-to-online-store endpoint.
let's open a GH issue for that, unless you want to fix it here

BTW: thanks for catching it!

nah, not here. that should be a sep issue and fix: pr

i'll take care of #4726 once this pr is merged. see draft here; will rebase and mark as ready for review.

robhowley · 2024-11-04T15:08:17Z

giving this a bump.

* use pydantic models to populate fastapi docs Signed-off-by: Rob Howley <howley.robert@gmail.com> * fix feature service feature assignment Signed-off-by: Rob Howley <howley.robert@gmail.com> * add response types, mostly none Signed-off-by: Rob Howley <howley.robert@gmail.com> --------- Signed-off-by: Rob Howley <howley.robert@gmail.com>

# [0.42.0](v0.41.0...v0.42.0) (2024-12-05) ### Bug Fixes * Add adapters for sqlite datetime conversion ([#4797](#4797)) ([e198b17](e198b17)) * Added grpcio extras to default feature-server image ([#4737](#4737)) ([e9cd373](e9cd373)) * Changing node version in release ([7089918](7089918)) * Feast create empty online table when FeatureView attribute online=False ([#4666](#4666)) ([237c453](237c453)) * Fix db store types in Operator CRD ([#4798](#4798)) ([f09339e](f09339e)) * Fix the config issue for postgres ([#4776](#4776)) ([a36f7e5](a36f7e5)) * Fixed example materialize-incremental and improved explanation ([#4734](#4734)) ([ca8a7ab](ca8a7ab)) * Fixed SparkSource docstrings so it wouldn't used inhereted class docstrings ([#4722](#4722)) ([32e6aa1](32e6aa1)) * Fixing PGVector integration tests ([#4778](#4778)) ([88a0320](88a0320)) * Incorrect type passed to assert_permissions in materialize endpoints ([#4727](#4727)) ([b72c2da](b72c2da)) * Issue of DataSource subclasses using parent abstract class docstrings ([#4730](#4730)) ([b24acd5](b24acd5)) * Operator envVar positioning & tls.SecretRef.Name ([#4806](#4806)) ([1115d96](1115d96)) * Populates project created_time correctly according to created ti… ([#4686](#4686)) ([a61b93c](a61b93c)) * Reduce feast-server container image size & fix dev image build ([#4781](#4781)) ([ccc9aea](ccc9aea)) * Removed version func from feature_store.py ([#4748](#4748)) ([f902bb9](f902bb9)) * Support registry instantiation for read-only users ([#4719](#4719)) ([ca3d3c8](ca3d3c8)) * Syntax Error in BigQuery While Retrieving Columns that Start wit… ([#4713](#4713)) ([60fbc62](60fbc62)) * Update release version in a pertinent Operator file ([#4708](#4708)) ([764a8a6](764a8a6)) ### Features * Add api contract to fastapi docs ([#4721](#4721)) ([1a165c7](1a165c7)) * Add Couchbase as an online store ([#4637](#4637)) ([824859b](824859b)) * Add Operator support for spec.feastProject & status.applied fields ([#4656](#4656)) ([430ac53](430ac53)) * Add services functionality to Operator ([#4723](#4723)) ([d1d80c0](d1d80c0)) * Add TLS support to the Operator ([#4796](#4796)) ([a617a6c](a617a6c)) * Added feast Go operator db stores support ([#4771](#4771)) ([3302363](3302363)) * Added support for setting env vars in feast services in feast controller ([#4739](#4739)) ([84b24b5](84b24b5)) * Adding docs outlining native Python transformations on singletons ([#4741](#4741)) ([0150278](0150278)) * Adding first feast operator e2e test. ([#4791](#4791)) ([8339f8d](8339f8d)) * Adding github action to run the operator end-to-end tests. ([#4762](#4762)) ([d8ccb00](d8ccb00)) * Adding ssl support for registry server. ([#4718](#4718)) ([ccf7a55](ccf7a55)) * Adding SSL support for the React UI server and feast UI command. ([#4736](#4736)) ([4a89252](4a89252)) * Adding support for native Python transformations on a single dictionary ([#4724](#4724)) ([9bbc1c6](9bbc1c6)) * Adding TLS support for offline server. ([#4744](#4744)) ([5d8d03f](5d8d03f)) * Building the feast image ([#4775](#4775)) ([6635dde](6635dde)) * File persistence definition and implementation ([#4742](#4742)) ([3bad4a1](3bad4a1)) * Object store persistence in operator ([#4758](#4758)) ([0ae86da](0ae86da)) * OIDC authorization in Feast Operator ([#4801](#4801)) ([eb111d6](eb111d6)) * Operator will create k8s serviceaccount for each feast service ([#4767](#4767)) ([cde5760](cde5760)) * Printing more verbose logs when we start the offline server ([#4660](#4660)) ([9d8d3d8](9d8d3d8)) * PVC configuration and impl ([#4750](#4750)) ([785a190](785a190)) * Qdrant vectorstore support ([#4689](#4689)) ([86573d2](86573d2)) * RBAC Authorization in Feast Operator ([#4786](#4786)) ([0ef5acc](0ef5acc)) * Support for nested timestamp fields in Spark Offline store ([#4740](#4740)) ([d4d94f8](d4d94f8)) * Update the go feature server from Expedia code repo. ([#4665](#4665)) ([6406625](6406625)) * Updated feast Go operator db stores ([#4809](#4809)) ([2c5a6b5](2c5a6b5)) * Updated sample secret following review ([#4811](#4811)) ([dc9f825](dc9f825))

use pydantic models to populate fastapi docs

ad98954

Signed-off-by: Rob Howley <howley.robert@gmail.com>

robhowley force-pushed the rh-api-docs branch from 10de18a to ad98954 Compare October 30, 2024 21:18

robhowley added 2 commits October 30, 2024 17:46

fix feature service feature assignment

1981a87

Signed-off-by: Rob Howley <howley.robert@gmail.com>

add response types, mostly none

720b6bf

Signed-off-by: Rob Howley <howley.robert@gmail.com>

robhowley commented Oct 30, 2024

View reviewed changes

robhowley marked this pull request as ready for review October 30, 2024 22:00

robhowley requested a review from a team as a code owner October 30, 2024 22:00

robhowley commented Oct 30, 2024

View reviewed changes

franciscojavierarceo reviewed Oct 30, 2024

View reviewed changes

robhowley requested a review from franciscojavierarceo October 31, 2024 16:09

franciscojavierarceo added the ok-to-test label Oct 31, 2024

dmartinol reviewed Oct 31, 2024

View reviewed changes

franciscojavierarceo approved these changes Nov 4, 2024

View reviewed changes

HaoXuAI merged commit 1a165c7 into feast-dev:master Nov 5, 2024
27 checks passed

robhowley deleted the rh-api-docs branch November 5, 2024 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add api contract to fastapi docs #4721

feat: Add api contract to fastapi docs #4721

robhowley commented Oct 30, 2024 •

edited

Loading

robhowley Oct 30, 2024 •

edited

Loading

robhowley Oct 30, 2024

franciscojavierarceo left a comment

robhowley commented Oct 31, 2024

franciscojavierarceo commented Oct 31, 2024

robhowley commented Oct 31, 2024

dmartinol Oct 31, 2024

dmartinol Oct 31, 2024

robhowley Oct 31, 2024

dmartinol Oct 31, 2024

robhowley Oct 31, 2024 •

edited

Loading

robhowley commented Nov 4, 2024

		feature_service: Optional[str] = None
		features: Optional[List[str]] = None

feat: Add api contract to fastapi docs #4721

feat: Add api contract to fastapi docs #4721

Conversation

robhowley commented Oct 30, 2024 • edited Loading

What this PR does / why we need it:

Which issue(s) this PR fixes:

Misc

robhowley Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

robhowley Oct 30, 2024

Choose a reason for hiding this comment

franciscojavierarceo left a comment

Choose a reason for hiding this comment

robhowley commented Oct 31, 2024

franciscojavierarceo commented Oct 31, 2024

robhowley commented Oct 31, 2024

dmartinol Oct 31, 2024

Choose a reason for hiding this comment

dmartinol Oct 31, 2024

Choose a reason for hiding this comment

robhowley Oct 31, 2024

Choose a reason for hiding this comment

dmartinol Oct 31, 2024

Choose a reason for hiding this comment

robhowley Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

robhowley commented Nov 4, 2024

robhowley commented Oct 30, 2024 •

edited

Loading

robhowley Oct 30, 2024 •

edited

Loading

robhowley Oct 31, 2024 •

edited

Loading