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): Sql registry search #105

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

wparsley-eg
Copy link

What this PR does / why we need it:

This adds a function to the SQL registry to allow searching in accordance with the parameters set by the AI workbench team.

You will notice that we're not doing this search in a SQL query. That is because the database schemas are a proto stored as a VARCHAR and a little bit of metadata. As such, we have to pull all potentially relevant objects from the DB and then filter in memory. This is not ideal, but it's the only approach that will work.

@@ -1063,6 +1066,80 @@ def create_project_if_not_exists(self, project):
if new_project:
self._set_last_updated_metadata(update_datetime, project)

def search(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was under the impression search should be implemented using full text search. Is pagination implemented in this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

About Full Text Search, is that expectation set with Workbench team?
About Pagination, We are planning to switch to Feast Remote Registry. How does that impact the functionality?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remote registry leverages, Sql Registry so we will have the functionality. We don't need to reimplement. Just method definition in remote registry should help until we contribute the functionality. Looking for answers to my questions above.

Copy link
Author

Choose a reason for hiding this comment

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

Full text search in this context means "feature view and project names".
That is, the names of all feature views and all projects should be searchable using this functionality. That's what's indicated by the UI design that has been given to us.

@@ -55,6 +55,8 @@ require (
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/goccy/go-json v0.10.2 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/gonuts/commander v0.1.0 // indirect
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert this.

@@ -173,6 +173,10 @@ github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiu
github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/gonuts/commander v0.1.0 h1:EcDTiVw9oAVORFjQOEOuHQqcl6OXMyTgELocTq6zJ0I=
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants