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

feat: Add the ability to list objects by tags #4246

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

tchughesiv
Copy link
Contributor

@tchughesiv tchughesiv commented May 31, 2024

What this PR does / why we need it:

In the feast registry & cli, add the ability to list objects by tags... same as the UI already supports.
Here are the object types with this new --tags capability -

  • data-sources (DataSource)
  • entities (Entity)
  • feature-services (FeatureService)
  • feature-views (FeatureView)
  • on-demand-feature-views (OnDemandFeatureView)
  • StreamFeatureView

Which issue(s) this PR fixes:

Fixes: #4245

e.g.

$ feast feature-views list --help
Usage: feast feature-views list [OPTIONS]

  List all feature views

Options:
  --tags TEXT  Filter by tags (e.g. --tags 'key:value' --tags 'key:value,
               key:value, ...'). Items return when ALL tags match.
  --help       Show this message and exit.
$ feast feature-views list --tags team:driver_performance              
NAME                       ENTITIES    TYPE
driver_hourly_stats        {'driver'}  FeatureView
driver_hourly_stats_fresh  {'driver'}  FeatureView
$ feast feature-views list
NAME                       ENTITIES    TYPE
driver_hourly_stats        {'driver'}  FeatureView
driver_hourly_stats_fresh  {'driver'}  FeatureView
transformed_conv_rate_fresh  {'driver'}  OnDemandFeatureView
transformed_conv_rate        {'driver'}  OnDemandFeatureView

@lokeshrangineni
Copy link
Contributor

/LGTM

@tokoko
Copy link
Collaborator

tokoko commented May 31, 2024

We should probably follow up (maybe in other PRs) with similar changes for other feast objects that have tags (entities, data sources). Seems odd to have this only for feature views.

@tchughesiv
Copy link
Contributor Author

tchughesiv commented May 31, 2024

@tokoko i didn't realize that about the other objects. maybe its better to go ahead and add to this PR? where can i find a definitive list of the objects with tags? any docs? or should i just parse the code?

@tokoko
Copy link
Collaborator

tokoko commented May 31, 2024

Yeah, changes to others in this PR would be great. All the objects in BaseRegistry are worth checking out, but I don't think all of them have a list command in cli, so might be a moot point for some for now. You can simply go ahead and implement it for all *_list methods in cli.py (feature view, data source, entity, feature service, on demand feature view).

@dmartinol
Copy link
Contributor

We should probably follow up (maybe in other PRs) with similar changes for other feast objects that have tags (entities, data sources). Seems odd to have this only for feature views.

@tokoko, I would also add that, since the direction is to expose authorized access through Feast services, we should include the same filtering by tags for all List APIs in RegistryServer. What do you think?

@tokoko
Copy link
Collaborator

tokoko commented Jun 3, 2024

@dmartinol Yeah, I guess that's implied. Whatever changes we make to BaseRegistry will have to be reflected in RegistryServer as well.

@tchughesiv tchughesiv marked this pull request as draft June 3, 2024 14:35
@tchughesiv tchughesiv force-pushed the tags-dev branch 2 times, most recently from 1b54fa0 to d80b733 Compare June 3, 2024 20:38
@tchughesiv tchughesiv changed the title feat: Add --tags filtering capability to the feast feature-views list cli command feat: Add tags filtering capability to the feast registry & cli Jun 14, 2024
@tchughesiv tchughesiv changed the title feat: Add tags filtering capability to the feast registry & cli feat: Add list by tags capability to the feast registry & cli Jun 14, 2024
@tchughesiv tchughesiv changed the title feat: Add list by tags capability to the feast registry & cli feat: Add the ability to list objects by tags to the feast registry & cli Jun 14, 2024
@tchughesiv tchughesiv force-pushed the tags-dev branch 2 times, most recently from bb2151f to 4bf1ce0 Compare June 14, 2024 16:31
@tchughesiv
Copy link
Contributor Author

@tokoko @dmartinol i've added filtering by tags to all feast objects that have the tags field.

@tchughesiv tchughesiv changed the title feat: Add the ability to list objects by tags to the feast registry & cli feat: In the feast registry & cli, add the ability to list objects by tags Jun 14, 2024
@tchughesiv tchughesiv changed the title feat: In the feast registry & cli, add the ability to list objects by tags feat: Add the ability to list objects by tags Jun 14, 2024
@tchughesiv tchughesiv marked this pull request as ready for review June 14, 2024 21:18
@tokoko
Copy link
Collaborator

tokoko commented Jun 15, 2024

@tchughesiv Looks good to me, but better place for tests is in test_universal_registry.py. That's the one that's run for all registry implementations.

@tchughesiv tchughesiv marked this pull request as draft June 15, 2024 19:45
@dmartinol
Copy link
Contributor

👍 @tchughesiv shouldn't we also update the documentation for the CLI options?
BTW: unless you think it's obvious, I would also explain that all tags must match to satisfy the filtering condition (ATM there's no way to select resources matching any of the given tags)

@tokoko
Copy link
Collaborator

tokoko commented Jun 17, 2024

@dmartinol I also had to check in the code that all tags had to match lol. Maybe it should have been, but wasn't obvious to me.

@tchughesiv tchughesiv marked this pull request as ready for review June 17, 2024 16:48
@tchughesiv
Copy link
Contributor Author

@dmartinol @tokoko thanks for the reviews. i've made the requested changes, including improved docs and additional tests in test_universal_registry.py

Copy link
Collaborator

@tokoko tokoko left a comment

Choose a reason for hiding this comment

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

LGTM

@franciscojavierarceo franciscojavierarceo enabled auto-merge (squash) June 17, 2024 23:58
auto-merge was automatically disabled June 18, 2024 14:56

Head branch was pushed to by a user without write access

Signed-off-by: Tommy Hughes <tohughes@redhat.com>
@jeremyary
Copy link
Collaborator

Thanks all for the reviews & feedback!

@jeremyary jeremyary merged commit fbf92da into feast-dev:master Jun 18, 2024
16 checks passed
franciscojavierarceo pushed a commit that referenced this pull request Jun 18, 2024
# [0.39.0](v0.38.0...v0.39.0) (2024-06-18)

### Bug Fixes

* Feast UI importlib change ([#4248](#4248)) ([5d486b8](5d486b8))
* Feature server no_feature_log argument error ([#4255](#4255)) ([15524ce](15524ce))
* Feature UI Server image won't start in an OpenShift cluster ([#4250](#4250)) ([4891f76](4891f76))
* Handles null values in data during GO Feature retrieval ([#4274](#4274)) ([c491e57](c491e57))
* Make Java gRPC client use timeouts as expected ([#4237](#4237)) ([f5a37c1](f5a37c1))
* Remove self assignment code line. ([#4238](#4238)) ([e514f66](e514f66))
* Set default values for feature_store.serve() function ([#4225](#4225)) ([fa74438](fa74438))

### Features

* Add online_read_async for dynamodb ([#4244](#4244)) ([b5ef384](b5ef384))
* Add the ability to list objects by `tags` ([#4246](#4246)) ([fbf92da](fbf92da))
* Added deadline to gRPC Java client ([#4217](#4217)) ([ff429c9](ff429c9))
* Adding vector search for sqlite ([#4176](#4176)) ([2478831](2478831))
* Change get_online_features signature, move online retrieval functions to utils ([#4278](#4278)) ([7287662](7287662))
* Feature/adding remote online store ([#4226](#4226)) ([9454d7c](9454d7c))
* List all feature views ([#4256](#4256)) ([36a574d](36a574d))
* Make RegistryServer writable ([#4231](#4231)) ([79e1143](79e1143))
* Remote offline Store  ([#4262](#4262)) ([28a3d24](28a3d24))
* Set optional full-scan for deletion ([#4189](#4189)) ([b9cadd5](b9cadd5))
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.

Add --tags filtering capability to the feast feature-views list cli command
6 participants