-
Notifications
You must be signed in to change notification settings - Fork 43
feat: search command #115
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: search command #115
Conversation
} | ||
defer conn.Close() | ||
|
||
query := args[0] |
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.
add check for args length. Otherwise it will panic if search query not given.
cmd/search.go
Outdated
strconv.Itoa(i + 1), | ||
h.GetNamespaceId(), | ||
h.GetSchemaId(), | ||
strings.Join(h.GetFields(), ","), |
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.
Output of search results are unreadable if more number of matching field names or messages present. Maybe we can print each field/type name in separate row to make it more user friendly.
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.
Right! How about this @harikrishnakanchi?
Also, I am thinking of removing the "NAMESPACE" and "SCHEMA" details as well, since their values will be the same as the user passed in flags.
Showing 2 result(s)
1)
FIELD: stencil.One.field_one
TYPE: stencil.One
VERSION: 1
2)
FIELD: stencil.One.field_one,stencil.One.Two.field_two
TYPE: stencil.One,stencil.One.Two
VERSION: 2
TOTAL
2
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.
something like this should be good.
Namespace | Schema | version | Type | Fields |
---|---|---|---|---|
org | schema-name | 1 | odpf.stencil.One | odpf.stencil.One.field_one, odpf.stencil.One.field_one |
With this we need to group fields by type name. Currently it just returns all matching fields and all matching type. Fields can be segregated by type then show it.
This is how this will look now. WDYT about it?
Shall we remove the NAMESPACE and SCHEMA columns, since it will be a piece of redundant information (same as flag value)? @harikrishnakanchi |
@ishanarya0 lets keep them. Also specifying namespace and schema should also be optional. User should be able to search across schemas. Does API support that right now? |
Yes, keeping them. Oh, yes they can be made optional too. Doing the changes. Thanks for pointing it out! |
Command:
$ stencil search <query> --namespace=<namespace> --schema=<schema> --version=<version> --history=<history>
Set either of
version
orhistory
flags. Version flag is used to search on a particular version while history flag enables the user to search across all the versions.