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.

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

feat(stencil): add support for search API #92

Merged
merged 3 commits into from
Feb 2, 2022
Merged

feat(stencil): add support for search API #92

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6c1819
feat(stencil): add support for search API
Jan 21, 2022
bb19300
feat(stencil): add global search api
Jan 28, 2022
c78ac24
feat(stencil): global search api
Feb 1, 2022
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
35 changes: 35 additions & 0 deletions odpf/stencil/v1beta1/stencil.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,16 @@ service StencilService {
summary: "Delete specified version of the schema";
};
}

rpc Search(SearchRequest) returns (SearchResponse) {
option (google.api.http) = {
get: "/v1beta1/search"
};
option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = {
tags: "schema";
summary: "Global Search API";
};
}
}

message Namespace {
Expand Down Expand Up @@ -295,3 +305,28 @@ message DeleteVersionRequest {
message DeleteVersionResponse {
string message = 1;
}

message SearchRequest {
string namespace_id = 1;
string schema_id = 2;
int32 version_id = 3;
string query = 4;
bool all = 5;
Copy link
Member

Choose a reason for hiding this comment

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

@ramey What is the role of all here?

Copy link
Member Author

Choose a reason for hiding this comment

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

all returns matches from all the versions, by default only the matches from the latest versions are returned

Copy link
Member

Choose a reason for hiding this comment

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

Make sense. Is there some better name we can think of for this? At first look I got confused that it is related to pagination,

Copy link
Member Author

Choose a reason for hiding this comment

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

how does scan_all_versions sound?

Copy link
Member

@h4rikris h4rikris Jan 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

what we need is oneof between version_id and all fields. User can't set both fields together right? If we do allow setting two fields then, which field should take precedence?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is while processing the value in the application. In case no value has been specified as it is a bool it defaults to false and there is no way to find out whether it is explicitly given by the user or default bool value.
Agreed with oneof, will change that.

Copy link
Member

@ravisuhag ravisuhag Jan 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

make sense. In that case, another name of all could be history/past as well.

Copy link
Member

Choose a reason for hiding this comment

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

@ramey @harikrishnakanchi lets go with history then?

Copy link
Member Author

@ramey ramey Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@ravisuhag I am thinking of going with history

Copy link
Member

Choose a reason for hiding this comment

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

@ramey cool.

}

message SearchResponse {
repeated SearchHits hits = 1;
SearchMeta meta = 2;
}

message SearchHits {
string namespace_id = 1;
string schema_id = 2;
int32 version_id = 3;
repeated string fields = 4;
repeated string types = 5;
}

message SearchMeta {
uint32 total = 1;
}