GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — feat: Add api contract to fastapi docs by robhowley · Pull Request #4721 · feast-dev/feast

Conversation

robhowley commented Oct 30, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          use pydantic models to populate fastapi docs

ad98954

Signed-off-by: Rob Howley <[email protected]>

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 <[email protected]>


          add response types, mostly none

720b6bf

Signed-off-by: Rob Howley <[email protected]>

robhowley commented

Oct 30, 2024

View reviewed changes

Comment thread

sdk/python/feast/feature_server.py

Comment on lines -119 to +121

		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]:

robhowley Oct 30, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

franciscojavierarceo reacted with hooray emoji

All reactions

🎉 1 reaction

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

Comment thread

sdk/python/feast/feature_server.py

Comment on lines +73 to +74

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

robhowley Oct 30, 2024

Copy link

Copy Markdown

Contributor Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

franciscojavierarceo reacted with thumbs up emoji

All reactions

👍 1 reaction

franciscojavierarceo reviewed

Oct 30, 2024

View reviewed changes

franciscojavierarceo left a comment

Copy link

Copy Markdown

Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

robhowley commented Oct 31, 2024

Copy link

Copy Markdown

Contributor Author

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 reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

robhowley requested a review from franciscojavierarceo

October 31, 2024 16:09

franciscojavierarceo commented Oct 31, 2024

Copy link

Copy Markdown

Member

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

robhowley reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

franciscojavierarceo added the ok-to-test label

Oct 31, 2024

robhowley commented Oct 31, 2024

Copy link

Copy Markdown

Contributor Author

flake?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

dmartinol reviewed

Oct 31, 2024

View reviewed changes

Comment thread

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?

dmartinol Oct 31, 2024

Copy link

Copy Markdown

Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dmartinol Oct 31, 2024

Copy link

Copy Markdown

Contributor

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.

BTW: thanks for catching it!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

robhowley reacted with thumbs up emoji

All reactions

👍 1 reaction

robhowley Oct 31, 2024

Copy link

Copy Markdown

Contributor Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

dmartinol reacted with thumbs up emoji

All reactions

👍 1 reaction

dmartinol Oct 31, 2024

Copy link

Copy Markdown

Contributor

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.

#4726

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

robhowley and franciscojavierarceo reacted with heart emoji

All reactions

❤️ 2 reactions

robhowley Oct 31, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

franciscojavierarceo reacted with hooray emoji

All reactions

🎉 1 reaction

robhowley commented Nov 4, 2024

Copy link

Copy Markdown

Contributor Author

giving this a bump.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

franciscojavierarceo approved these changes

Nov 4, 2024

View reviewed changes

haoxu0 merged commit 1a165c7 into feast-dev:master

Nov 5, 2024

robhowley deleted the rh-api-docs branch

November 5, 2024 20:11

shuchu pushed a commit to shuchu/feast that referenced this pull request

Nov 21, 2024


          feat: Add api contract to fastapi docs (feast-dev#4721)

c0fe77a

* use pydantic models to populate fastapi docs

Signed-off-by: Rob Howley <[email protected]>

* fix feature service feature assignment

Signed-off-by: Rob Howley <[email protected]>

* add response types, mostly none

Signed-off-by: Rob Howley <[email protected]>

---------

Signed-off-by: Rob Howley <[email protected]>

franciscojavierarceo pushed a commit that referenced this pull request

Dec 5, 2024


          chore(release): release 0.42.0

aac334a

# [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))

dharmisha pushed a commit to nishantgaurav-dev/feast that referenced this pull request

Jan 15, 2025


          chore(release): release 0.42.0

6c64b8f

* Add adapters for sqlite datetime conversion ([feast-dev#4797](feast-dev#4797)) ([e198b17](feast-dev@e198b17))
* Added grpcio extras to default feature-server image ([feast-dev#4737](feast-dev#4737)) ([e9cd373](feast-dev@e9cd373))
* Changing node version in release ([7089918](feast-dev@7089918))
* Feast create empty online table when FeatureView attribute online=False ([feast-dev#4666](feast-dev#4666)) ([237c453](feast-dev@237c453))
* Fix db store types in Operator CRD ([feast-dev#4798](feast-dev#4798)) ([f09339e](feast-dev@f09339e))
* Fix the config issue for postgres ([feast-dev#4776](feast-dev#4776)) ([a36f7e5](feast-dev@a36f7e5))
* Fixed example materialize-incremental and improved explanation ([feast-dev#4734](feast-dev#4734)) ([ca8a7ab](feast-dev@ca8a7ab))
* Fixed SparkSource docstrings so it wouldn't used inhereted class docstrings ([feast-dev#4722](feast-dev#4722)) ([32e6aa1](feast-dev@32e6aa1))
* Fixing PGVector integration tests ([feast-dev#4778](feast-dev#4778)) ([88a0320](feast-dev@88a0320))
* Incorrect type passed to assert_permissions in materialize endpoints ([feast-dev#4727](feast-dev#4727)) ([b72c2da](feast-dev@b72c2da))
* Issue of DataSource subclasses using parent abstract class docstrings ([feast-dev#4730](feast-dev#4730)) ([b24acd5](feast-dev@b24acd5))
* Operator envVar positioning & tls.SecretRef.Name ([feast-dev#4806](feast-dev#4806)) ([1115d96](feast-dev@1115d96))
* Populates project created_time correctly according to created ti… ([feast-dev#4686](feast-dev#4686)) ([a61b93c](feast-dev@a61b93c))
* Reduce feast-server container image size & fix dev image build ([feast-dev#4781](feast-dev#4781)) ([ccc9aea](feast-dev@ccc9aea))
* Removed version func from feature_store.py ([feast-dev#4748](feast-dev#4748)) ([f902bb9](feast-dev@f902bb9))
* Support registry instantiation for read-only users ([feast-dev#4719](feast-dev#4719)) ([ca3d3c8](feast-dev@ca3d3c8))
* Syntax Error in BigQuery While Retrieving Columns that Start wit… ([feast-dev#4713](feast-dev#4713)) ([60fbc62](feast-dev@60fbc62))
* Update release version in a pertinent Operator file ([feast-dev#4708](feast-dev#4708)) ([764a8a6](feast-dev@764a8a6))

* Add api contract to fastapi docs ([feast-dev#4721](feast-dev#4721)) ([1a165c7](feast-dev@1a165c7))
* Add Couchbase as an online store ([feast-dev#4637](feast-dev#4637)) ([824859b](feast-dev@824859b))
* Add Operator support for spec.feastProject & status.applied fields ([feast-dev#4656](feast-dev#4656)) ([430ac53](feast-dev@430ac53))
* Add services functionality to Operator ([feast-dev#4723](feast-dev#4723)) ([d1d80c0](feast-dev@d1d80c0))
* Add TLS support to the Operator ([feast-dev#4796](feast-dev#4796)) ([a617a6c](feast-dev@a617a6c))
* Added feast Go operator db stores support ([feast-dev#4771](feast-dev#4771)) ([3302363](feast-dev@3302363))
* Added support for setting env vars in feast services in feast controller  ([feast-dev#4739](feast-dev#4739)) ([84b24b5](feast-dev@84b24b5))
* Adding docs outlining native Python transformations on singletons ([feast-dev#4741](feast-dev#4741)) ([0150278](feast-dev@0150278))
* Adding first feast operator e2e test. ([feast-dev#4791](feast-dev#4791)) ([8339f8d](feast-dev@8339f8d))
* Adding github action to run the operator end-to-end tests. ([feast-dev#4762](feast-dev#4762)) ([d8ccb00](feast-dev@d8ccb00))
* Adding ssl support for registry server. ([feast-dev#4718](feast-dev#4718)) ([ccf7a55](feast-dev@ccf7a55))
* Adding SSL support for the React UI server and feast UI command. ([feast-dev#4736](feast-dev#4736)) ([4a89252](feast-dev@4a89252))
* Adding support for native Python transformations on a single dictionary  ([feast-dev#4724](feast-dev#4724)) ([9bbc1c6](feast-dev@9bbc1c6))
* Adding TLS support for offline server. ([feast-dev#4744](feast-dev#4744)) ([5d8d03f](feast-dev@5d8d03f))
* Building the feast image ([feast-dev#4775](feast-dev#4775)) ([6635dde](feast-dev@6635dde))
* File persistence definition and implementation ([feast-dev#4742](feast-dev#4742)) ([3bad4a1](feast-dev@3bad4a1))
* Object store persistence in operator ([feast-dev#4758](feast-dev#4758)) ([0ae86da](feast-dev@0ae86da))
* OIDC authorization in Feast Operator ([feast-dev#4801](feast-dev#4801)) ([eb111d6](feast-dev@eb111d6))
* Operator will create k8s serviceaccount for each feast service ([feast-dev#4767](feast-dev#4767)) ([cde5760](feast-dev@cde5760))
* Printing more verbose logs when we start the offline server  ([feast-dev#4660](feast-dev#4660)) ([9d8d3d8](feast-dev@9d8d3d8))
* PVC configuration and impl ([feast-dev#4750](feast-dev#4750)) ([785a190](feast-dev@785a190))
* Qdrant vectorstore support ([feast-dev#4689](feast-dev#4689)) ([86573d2](feast-dev@86573d2))
* RBAC Authorization in Feast Operator ([feast-dev#4786](feast-dev#4786)) ([0ef5acc](feast-dev@0ef5acc))
* Support for nested timestamp fields in Spark Offline store ([feast-dev#4740](feast-dev#4740)) ([d4d94f8](feast-dev@d4d94f8))
* Update the go feature server from Expedia code repo. ([feast-dev#4665](feast-dev#4665)) ([6406625](feast-dev@6406625))
* Updated feast Go operator db stores ([feast-dev#4809](feast-dev#4809)) ([2c5a6b5](feast-dev@2c5a6b5))
* Updated sample secret following review ([feast-dev#4811](feast-dev#4811)) ([dc9f825](feast-dev@dc9f825))

Reviewers

dmartinol

dmartinol left review comments

franciscojavierarceo

franciscojavierarceo approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Addition of /docs endpoint

Uh oh!

There was an error while loading. Please reload this page.

4 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

robhowley commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Misc

Uh oh!

robhowley Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robhowley Oct 30, 2024

Choose a reason for hiding this comment

Uh oh!

franciscojavierarceo left a comment

Choose a reason for hiding this comment

Uh oh!

robhowley commented Oct 31, 2024

Uh oh!

franciscojavierarceo commented Oct 31, 2024

Uh oh!

robhowley commented Oct 31, 2024

Uh oh!

dmartinol Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

dmartinol Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

robhowley Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

dmartinol Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

robhowley Oct 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robhowley commented Nov 4, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

robhowley commented Oct 30, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

robhowley Oct 30, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

robhowley Oct 31, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.