Skip to content
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.

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

docs: Add missing unpacking operator in Permission objects docs #4884

Conversation

boliri
Copy link
Contributor

@boliri boliri commented Jan 2, 2025

What this PR does / why we need it:

The current example for setting up a Permission object with the READ action, which serves as a composition of multiple read actions (READ_OFFLINE and READ_ONLINE), makes Feast crash when running feast apply.

It is documented like this at the moment:

from feast.permissions.action import AuthzedAction, READ

Permission(
    name="feature-reader",
    types=[FeatureView, FeatureService],
    policy=RoleBasedPolicy(roles=["super-reader"]),
    actions=[AuthzedAction.DESCRIBE, READ],
)

So actions ultimately ends up resolving READ and looking like this:

from feast.permissions.action import AuthzedAction, READ

Permission(
    name="feature-reader",
    types=[FeatureView, FeatureService],
    policy=RoleBasedPolicy(roles=["super-reader"]),
    actions=[AuthzedAction.DESCRIBE, [AuthzedAction.READ_OFFLINE, AuthzedAction.READ_ONLINE]],
)

And feast apply yields the following exception when trying to persist that permission to the Registry:

$ feast apply

Applying changes for project feast-demo

Traceback (most recent call last):
  File "/usr/local/bin/feast", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/click/decorators.py", line 33, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/feast/cli.py", line 760, in apply_total_command
    apply_total(repo_config, repo, skip_source_validation)
  File "/usr/local/lib/python3.11/site-packages/feast/repo_operations.py", line 404, in apply_total
    apply_total_with_repo_instance(
  File "/usr/local/lib/python3.11/site-packages/feast/repo_operations.py", line 351, in apply_total_with_repo_instance
    store.apply(all_to_apply, objects_to_delete=all_to_delete, partial=False)
  File "/usr/local/lib/python3.11/site-packages/feast/feature_store.py", line 960, in apply
    self._registry.apply_permission(
  File "/usr/local/lib/python3.11/site-packages/feast/infra/registry/sql.py", line 1176, in apply_permission
    return self._apply_object(
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/feast/infra/registry/sql.py", line 969, in _apply_object
    proto_field_name: obj.to_proto().SerializeToString(),
                      ^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/feast/permissions/permission.py", line 218, in to_proto
    actions = [
              ^
  File "/usr/local/lib/python3.11/site-packages/feast/permissions/permission.py", line 219, in <listcomp>
    PermissionSpecProto.AuthzedAction.Value(action.name)
                                            ^^^^^^^^^^^
AttributeError: 'list' object has no attribute 'name'

We need the unpacking operator (*) to flatten the READ list and make the Permission look like this at the end:

from feast.permissions.action import AuthzedAction, READ

Permission(
    name="feature-reader",
    types=[FeatureView, FeatureService],
    policy=RoleBasedPolicy(roles=["super-reader"]),
    actions=[AuthzedAction.DESCRIBE, *READ],
)

Which resolves as:

from feast.permissions.action import AuthzedAction, READ

Permission(
    name="feature-reader",
    types=[FeatureView, FeatureService],
    policy=RoleBasedPolicy(roles=["super-reader"]),
    actions=[AuthzedAction.DESCRIBE, AuthzedAction.READ_OFFLINE, AuthzedAction.READ_ONLINE],
)

@boliri boliri requested a review from a team as a code owner January 2, 2025 13:28
@boliri boliri changed the title docs: add missing unpacking operator in Permission objects docs docs: Add missing unpacking operator in Permission objects docs Jan 2, 2025
@boliri boliri force-pushed the fix/missing-unpacking-operator-in-permission-docs branch from f52b392 to e17fa25 Compare January 2, 2025 13:32
Copy link
Contributor

@dmartinol dmartinol left a comment

Choose a reason for hiding this comment

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

Thank you

@dmartinol dmartinol enabled auto-merge (squash) January 9, 2025 15:51
@franciscojavierarceo franciscojavierarceo merged commit 1d47cb6 into feast-dev:master Jan 9, 2025
15 checks passed
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.

3 participants