Skip to content

feat: Add api contract to fastapi docs #4721

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.

Sign up for GitHub

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

Merged
HaoXuAI merged 3 commits into from
Nov 5, 2024
Merged

feat: Add api contract to fastapi docs #4721

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad98954
use pydantic models to populate fastapi docs
robhowley Oct 30, 2024
1981a87
fix feature service feature assignment
robhowley Oct 30, 2024
720b6bf
add response types, mostly none
robhowley Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add response types, mostly none 
Signed-off-by: Rob Howley <howley.robert@gmail.com>
  • Loading branch information
@robhowley
robhowley committed Oct 30, 2024
commit 720b6bf2fdd135a4fc5d8ed46c57c236c8270b49
10 changes: 5 additions & 5 deletions sdk/python/feast/feature_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ async def lifespan(app: FastAPI):
"/get-online-features",
dependencies=[Depends(inject_user_details)],
)
async def get_online_features(request: GetOnlineFeaturesRequest):
async def get_online_features(request: GetOnlineFeaturesRequest) -> Dict[str, Any]:
# Initialize parameters for FeatureStore.get_online_features(...) call
if request.feature_service:
feature_service = store.get_feature_service(
Expand Down Expand Up @@ -167,7 +167,7 @@ async def get_online_features(request: GetOnlineFeaturesRequest):
)

@app.post("/push", dependencies=[Depends(inject_user_details)])
async def push(request: PushFeaturesRequest):
async def push(request: PushFeaturesRequest) -> None:
df = pd.DataFrame(request.df)
actions = []
if request.to == "offline":
Expand Down Expand Up @@ -219,7 +219,7 @@ async def push(request: PushFeaturesRequest):
store.push(**push_params)

@app.post("/write-to-online-store", dependencies=[Depends(inject_user_details)])
def write_to_online_store(request: WriteToFeatureStoreRequest):
def write_to_online_store(request: WriteToFeatureStoreRequest) -> None:
df = pd.DataFrame(request.df)
feature_view_name = request.feature_view_name
allow_registry_cache = request.allow_registry_cache
Expand Down Expand Up @@ -248,7 +248,7 @@ async def health():
)

@app.post("/materialize", dependencies=[Depends(inject_user_details)])
def materialize(request: MaterializeRequest):
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?
Copy link
Contributor

@dmartinol dmartinol Oct 31, 2024

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

Copy link
Contributor

@dmartinol dmartinol Oct 31, 2024

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!

Copy link
Contributor Author

@robhowley robhowley Oct 31, 2024

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

Copy link
Contributor

@dmartinol dmartinol Oct 31, 2024

Choose a reason for hiding this comment

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

#4726

Copy link
Contributor Author

@robhowley robhowley Oct 31, 2024 •
edited
Loading

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.

assert_permissions(
Expand All @@ -262,7 +262,7 @@ def materialize(request: MaterializeRequest):
)

@app.post("/materialize-incremental", dependencies=[Depends(inject_user_details)])
def materialize_incremental(request: MaterializeIncrementalRequest):
def materialize_incremental(request: MaterializeIncrementalRequest) -> 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?
assert_permissions(
Expand Down