Skip to content

fix: Incorrect type passed to assert_permissions in materialize endpoints #4727

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
franciscojavierarceo merged 1 commit into from
Nov 5, 2024
Merged

fix: Incorrect type passed to assert_permissions in materialize endpoints #4727

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 15 additions & 12 deletions sdk/python/feast/feature_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
FeastError,
FeatureViewNotFoundException,
)
from feast.feast_object import FeastObject
from feast.permissions.action import WRITE, AuthzedAction
from feast.permissions.security_manager import assert_permissions
from feast.permissions.server.rest import inject_user_details
Expand Down Expand Up @@ -218,21 +219,25 @@ async def push(request: PushFeaturesRequest) -> None:
else:
store.push(**push_params)

@app.post("/write-to-online-store", dependencies=[Depends(inject_user_details)])
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
def _get_feast_object(
feature_view_name: str, allow_registry_cache: bool
) -> FeastObject:
try:
feature_view = store.get_stream_feature_view( # type: ignore
return store.get_stream_feature_view( # type: ignore
feature_view_name, allow_registry_cache=allow_registry_cache
)
except FeatureViewNotFoundException:
feature_view = store.get_feature_view( # type: ignore
return store.get_feature_view( # type: ignore
feature_view_name, allow_registry_cache=allow_registry_cache
)

assert_permissions(resource=feature_view, actions=[AuthzedAction.WRITE_ONLINE])
@app.post("/write-to-online-store", dependencies=[Depends(inject_user_details)])
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
resource = _get_feast_object(feature_view_name, allow_registry_cache)
assert_permissions(resource=resource, actions=[AuthzedAction.WRITE_ONLINE])
store.write_to_online_store(
feature_view_name=feature_view_name,
df=df,
Expand All @@ -250,9 +255,8 @@ async def health():
@app.post("/materialize", dependencies=[Depends(inject_user_details)])
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?
assert_permissions(
resource=feature_view, # type: ignore
resource=_get_feast_object(feature_view, True),
actions=[AuthzedAction.WRITE_ONLINE],
)
Comment on lines -253 to 261
Copy link
Contributor Author

@robhowley robhowley Nov 5, 2024

Choose a reason for hiding this comment

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

fix: we don't assert_permissions on the str name of the feature_view, but on the FeastObject itself

store.materialize(
Expand All @@ -264,9 +268,8 @@ def materialize(request: MaterializeRequest) -> None:
@app.post("/materialize-incremental", dependencies=[Depends(inject_user_details)])
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(
resource=feature_view, # type: ignore
resource=_get_feast_object(feature_view, True),
actions=[AuthzedAction.WRITE_ONLINE],
)
store.materialize_incremental(
Expand Down
Loading