-
Notifications
You must be signed in to change notification settings - Fork 17
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
User-friendly operators #356
Milestone
Comments
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Signed-off-by: Denys Fedoryshchenko <[email protected]>
I sent basic PR that will address at least undocumented features and make them more easy to document. |
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 18, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Also raise error if attribute name is used twice, as first one will be overwritten and ignored due current implementation. Signed-off-by: Denys Fedoryshchenko <[email protected]>
nuclearcat
added a commit
to nuclearcat/kernelci-core
that referenced
this issue
Sep 19, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Also raise error if attribute name is used twice, as first one will be overwritten and ignored due current implementation. Signed-off-by: Denys Fedoryshchenko <[email protected]>
github-merge-queue bot
pushed a commit
to kernelci/kernelci-core
that referenced
this issue
Oct 3, 2023
Right now for operators we are using "magic" suffix such as __gt, __lt and so on, which is not documented and a bit non-intuitive. We can make cli tool more intuitive with this patch, where it will hide from user this magic, until we find better mechanism to handle operators as mentioned in kernelci/kernelci-api#356 Also raise error if attribute name is used twice, as first one will be overwritten and ignored due current implementation. Signed-off-by: Denys Fedoryshchenko <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The current syntax for operators on the command line is the same as in the API URLs, with a
__
separator between the attribute name and an operator. For example, to count the number of kernel revisions (checkout
nodes) since the beginning of the Early Access phase:This works, however there are a few things that could be improved:
Combining operators doesn't work
This should return the number of nodes for the 1-day period, but instead it only takes into account the 2nd query parameter:
Deal with attributes that have
__
in their nameSome test suites might well have
__
in their name or particular jobs might generate custom data including it in their attributes. While we could make it invalid and the API would be rejecting such data, we could also look for other ways to encode the operators in the URLs. At the very least, we could just make it invalid to name attributes that end with an operator name. For example,foo_bar
would still be valid, and evenfoo__bar__baz
, but notfoo_gte
orfoo_bar_lt
asgte
andlt
as known operators.Consider additional operators
The current 4 operators were added out of necessity for the initial use-cases we had. A fully-fledged system would need to be able to deal with a broader range of use cases and as such more operators are likely to be required. See the list of MongoDB operators as a reference, they could all be implemented very easily since internally the API uses MongoDB (at least the comparison ones).
Probably we could already just add the
ne
(not equal) operator to the list, in fact it's quite surprising we got this far without needing it.Command-line syntax
While some improvements can be made on the API side to provide features and define any constraints with attribute names, the CLI syntax offered by
kci
could also be made simpler. Right now, it's quite easy to forget one underscore and doingcreated_lt=2024
will not match anything as that's looking for an attribute calledcreated_lt
.If we wanted to keep it as-is and just pass the search parameters to the URL, we could have a check in
kci
if the attribute name ends with a known operator and print a warning to the user.On top of this, we could also provide some symbols such as
<
,<=
,==
etc. as equivalents to the operator acronyms. A sample command line would then look like this:Please note that this would involve lots of characters that aren't very usable on the command line:
<
and>
are used for redirecting streams,!
is a magic character in most shells to do things like running a previous command and=
is used by parsers for arguments that take a value. So while making the queries more human-readable on the CLI is a good idea, it doesn't seem obviously better to just use these symbols. One option would be to create an interactive parser e.g.kci shell
and then any characters could be used without interfering the main OS shell.The text was updated successfully, but these errors were encountered: