-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: master
Are you sure you want to change the base?
Conversation
@@ -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( |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this.
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.