base_api.py: Implement operators and document them in cli help #2109

nuclearcat · 2023-09-18T10:36:24Z

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

Example tested:

kci node find 'created>2023-09-17' 'state=done' 'group=kver'

Fixes: kernelci/kernelci-api#356

gctucker · 2023-09-18T11:11:08Z

kci node find 'created>2023-09-17' 'state=done' 'group=kver'

Out of interest, would this work too? Since quotes are required anyway, we could also accept spaces around the operators. I guess it would just require a small tweak in the regex.

kci node find "created > 2023-09-17" "state = done" "group = kver"

gctucker · 2023-09-18T11:12:11Z

Ah also we need to make sure the previous syntax still works with e.g. created__gt=2023-09-17 etc. I think it should by looking at the code, but it's a bit implicit.

nuclearcat · 2023-09-18T11:28:54Z

I forgot that variables might contain special characters too, just \w is not enough, i updated it, but i think still need to verify what kind of values might exist, as even new pattern might not match some special characters
Also now it is handling correctly space, and sure it handles old commands as well. Btw it is gte, not ge, this is what i mean, that mongodb operators are a bit harder to remember, than basic logical operators.

nuclearcat · 2023-09-18T12:48:51Z

            # This pattern allows spaces around the operator, e.g.:
            #   'foo = bar', but it create limitations in the name and value
            # name can't end with space and value can't start with space
            #pattern = r'^(.+\S)\s*([!=<>]+)\s*(\S.+)$'

I will leave it here and create PR without spaces support, as they add additional limitations on names and values.

kernelci/cli/base_api.py

nuclearcat · 2023-09-18T21:48:41Z

I added also verification, for case you mentioned before:

./kci node find "group=kver" "group=abc"
Traceback (most recent call last):
  File "/home/nuclearcat/Documents/kernelci-core/./kci", line 29, in <module>
    kernelci.cli.call(args.command, sys.argv[2:])
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/__init__.py", line 53, in call
    _COMMANDS[name](args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/node.py", line 71, in main
    sub_main("node", globals(), args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base.py", line 775, in sub_main
    status = opts.command(configs, opts)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base.py", line 417, in call
    return func(*args, **kwargs)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base_api.py", line 58, in __call__
    return self._api_call(api, configs, args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/node.py", line 38, in _api_call
    attributes = self._split_attributes(args.attributes)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base_api.py", line 99, in _split_attributes
    raise ValueError(f"Attribute {attribute} already exists")
ValueError: Attribute group already exists

Because many might not be aware because of our implementation details we cannot set in conditions same attribute name twice.

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 <denys.f@collabora.com>

nuclearcat · 2023-10-03T07:40:21Z

Tested locally:

/kci node find 'created>2023-09-01' --limit 1
[
    {
        "id": "64f12e0b8326c545a780b982",
...
        "created": "2023-09-01T00:19:23.219000",

gctucker · 2023-10-03T08:19:20Z

Has this been tested with the staging API instance? I think this got merged too quickly.

@JenySadadia Have you had a chance to look at this?

JenySadadia · 2023-10-03T08:45:54Z

Has this been tested with the staging API instance? I think this got merged too quickly.

@JenySadadia Have you had a chance to look at this?

Not yet. I'll test it soon.

JenySadadia

Tested on staging.

Attributes with = works without single quotes around.

$ ./kci node find name=checkout created=2023-07-17T11:52:31.913000
[{"id": "64b52b7fb5b4a9b8c05e0874", "kind": "node", "name": "checkout", "path": ["checkout"], "group": null, "revision": {"tree": "kernelci", "url": "https://github.com/kernelci/linux.git", "branch": "staging-mainline", "commit": "273cecedf983df3b9cc3fba85b773be154852975", "describe": "staging-mainline-20230717.0", "version": {"version": 6, "patchlevel": 5, "sublevel": null, "extra": "-rc2-1-g273cecedf983", "name": null}}, "parent": null, "state": "done", "result": null, "artifacts": {"tarball": "http://storage.staging.kernelci.org/api/linux-kernelci-staging-mainline-staging-mainline-20230717.0.tar.gz"}, "data": null, "created": "2023-07-17T11:52:31.913000", "updated": "2023-07-17T12:05:29.780000", "timeout": "2023-07-17T12:52:31.894000", "holdoff": "2023-07-17T12:03:49.144000", "owner": "staging.kernelci.org", "user_groups": []}]

However, it throws ValueError with other operators.

$ ./kci node find name=checkout created>2023-07-17T11:52:31.913000
Traceback (most recent call last):
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/./kci", line 29, in <module>
    kernelci.cli.call(args.command, sys.argv[2:])
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/__init__.py", line 53, in call
    _COMMANDS[name](args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/node.py", line 71, in main
    sub_main("node", globals(), args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base.py", line 791, in sub_main
    status = opts.command(configs, opts)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base.py", line 417, in call
    return func(*args, **kwargs)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base_api.py", line 58, in __call__
    return self._api_call(api, configs, args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/node.py", line 38, in _api_call
    attributes = self._split_attributes(args.attributes)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base_api.py", line 103, in _split_attributes
    raise ValueError(f"Invalid attribute {attribute}")
ValueError: Invalid attribute created

I'd suggest returning a proper error message instead of ValueError in such case @nuclearcat

JenySadadia

We should also enable the support for != operator. But that is not implemented in the API yet.

JenySadadia · 2023-10-03T09:47:26Z

I added also verification, for case you mentioned before:

./kci node find "group=kver" "group=abc"
Traceback (most recent call last):
  File "/home/nuclearcat/Documents/kernelci-core/./kci", line 29, in <module>
    kernelci.cli.call(args.command, sys.argv[2:])
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/__init__.py", line 53, in call
    _COMMANDS[name](args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/node.py", line 71, in main
    sub_main("node", globals(), args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base.py", line 775, in sub_main
    status = opts.command(configs, opts)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base.py", line 417, in call
    return func(*args, **kwargs)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base_api.py", line 58, in __call__
    return self._api_call(api, configs, args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/node.py", line 38, in _api_call
    attributes = self._split_attributes(args.attributes)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base_api.py", line 99, in _split_attributes
    raise ValueError(f"Attribute {attribute} already exists")
ValueError: Attribute group already exists

Because many might not be aware because of our implementation details we cannot set in conditions same attribute name twice.

It seems not working when I provided created twice.

$ ./kci node find name=checkout 'created>2023-07-17T11:53:31.913000' 'created=2023-07-17T11:52:31.913000'
[{"id": "64b52b7fb5b4a9b8c05e0874", "kind": "node", "name": "checkout", "path": ["checkout"], "group": null, "revision": {"tree": "kernelci", "url": "https://github.com/kernelci/linux.git", "branch": "staging-mainline", "commit": "273cecedf983df3b9cc3fba85b773be154852975", "describe": "staging-mainline-20230717.0", "version": {"version": 6, "patchlevel": 5, "sublevel": null, "extra": "-rc2-1-g273cecedf983", "name": null}}, "parent": null, "state": "done", "result": null, "artifacts": {"tarball": "http://storage.staging.kernelci.org/api/linux-kernelci-staging-mainline-staging-mainline-20230717.0.tar.gz"}, "data": null, "created": "2023-07-17T11:52:31.913000", "updated": "2023-07-17T12:05:29.780000", "timeout": "2023-07-17T12:52:31.894000", "holdoff": "2023-07-17T12:03:49.144000", "owner": "staging.kernelci.org", "user_groups": []}]

gctucker · 2023-10-03T10:27:27Z

The issue when providing the same attribute name twice is mentioned on kernelci/kernelci-api#356. Probably it would be good to have a checklist on that issue with things to do / fix before it's resolved.

r-c-n · 2023-10-03T10:43:38Z

However, it throws ValueError with other operators.

$ ./kci node find name=checkout created>2023-07-17T11:52:31.913000
Traceback (most recent call last):
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/./kci", line 29, in <module>
    kernelci.cli.call(args.command, sys.argv[2:])
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/__init__.py", line 53, in call
    _COMMANDS[name](args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/node.py", line 71, in main
    sub_main("node", globals(), args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base.py", line 791, in sub_main
    status = opts.command(configs, opts)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base.py", line 417, in call
    return func(*args, **kwargs)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base_api.py", line 58, in __call__
    return self._api_call(api, configs, args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/node.py", line 38, in _api_call
    attributes = self._split_attributes(args.attributes)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base_api.py", line 103, in _split_attributes
    raise ValueError(f"Invalid attribute {attribute}")
ValueError: Invalid attribute created

>, < and possibly ! will always need to be quoted or escaped so that the shell won't interpret them as operators, but the same is true of any other character that matches a shell operator.

nuclearcat · 2023-10-03T10:54:26Z

I added also verification, for case you mentioned before:

./kci node find "group=kver" "group=abc"
Traceback (most recent call last):
  File "/home/nuclearcat/Documents/kernelci-core/./kci", line 29, in <module>
    kernelci.cli.call(args.command, sys.argv[2:])
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/__init__.py", line 53, in call
    _COMMANDS[name](args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/node.py", line 71, in main
    sub_main("node", globals(), args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base.py", line 775, in sub_main
    status = opts.command(configs, opts)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base.py", line 417, in call
    return func(*args, **kwargs)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base_api.py", line 58, in __call__
    return self._api_call(api, configs, args)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/node.py", line 38, in _api_call
    attributes = self._split_attributes(args.attributes)
  File "/home/nuclearcat/Documents/kernelci-core/kernelci/cli/base_api.py", line 99, in _split_attributes
    raise ValueError(f"Attribute {attribute} already exists")
ValueError: Attribute group already exists

Because many might not be aware because of our implementation details we cannot set in conditions same attribute name twice.

It seems not working when I provided created twice.

Because of way how conditions is implemented in kernelci-api (suffix in attribute name, __gt and etc), if you put different conditions - it will not be same attribute name anymore.

JenySadadia · 2023-10-03T11:19:49Z

However, it throws ValueError with other operators.

$ ./kci node find name=checkout created>2023-07-17T11:52:31.913000
Traceback (most recent call last):
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/./kci", line 29, in <module>
    kernelci.cli.call(args.command, sys.argv[2:])
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/__init__.py", line 53, in call
    _COMMANDS[name](args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/node.py", line 71, in main
    sub_main("node", globals(), args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base.py", line 791, in sub_main
    status = opts.command(configs, opts)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base.py", line 417, in call
    return func(*args, **kwargs)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base_api.py", line 58, in __call__
    return self._api_call(api, configs, args)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/node.py", line 38, in _api_call
    attributes = self._split_attributes(args.attributes)
  File "/home/jeny/kernelCI/kernelci-core_env/src/kernelci-core/kernelci/cli/base_api.py", line 103, in _split_attributes
    raise ValueError(f"Invalid attribute {attribute}")
ValueError: Invalid attribute created

>, < and possibly ! will always need to be quoted or escaped so that the shell won't interpret them as operators, but the same is true of any other character that matches a shell operator.

That's true. I just recommended showing a proper error message to the user instead of returning the exception.

nuclearcat force-pushed the add-node-operators branch from 5dad6bf to 1d4aff3 Compare September 18, 2023 10:38

nuclearcat force-pushed the add-node-operators branch from 1d4aff3 to fd92f31 Compare September 18, 2023 11:24

nuclearcat force-pushed the add-node-operators branch from fd92f31 to 5c8b797 Compare September 18, 2023 12:49

gctucker reviewed Sep 18, 2023

View reviewed changes

kernelci/cli/base_api.py Outdated Show resolved Hide resolved

kernelci/cli/base_api.py Outdated Show resolved Hide resolved

kernelci/cli/base_api.py Outdated Show resolved Hide resolved

nuclearcat force-pushed the add-node-operators branch 2 times, most recently from 895ad07 to 6d50193 Compare September 18, 2023 21:47

nuclearcat force-pushed the add-node-operators branch from 6d50193 to 0139523 Compare September 18, 2023 21:49

nuclearcat force-pushed the add-node-operators branch from 0139523 to ab63b8a Compare September 19, 2023 05:58

nuclearcat requested a review from gctucker September 22, 2023 06:44

nuclearcat added this pull request to the merge queue Oct 3, 2023

Merged via the queue into kernelci:main with commit fb81830 Oct 3, 2023
4 checks passed

nuclearcat deleted the add-node-operators branch October 3, 2023 07:41

JenySadadia reviewed Oct 3, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

base_api.py: Implement operators and document them in cli help #2109

base_api.py: Implement operators and document them in cli help #2109

nuclearcat commented Sep 18, 2023 •

edited by gctucker

Loading

gctucker commented Sep 18, 2023

gctucker commented Sep 18, 2023 •

edited

Loading

nuclearcat commented Sep 18, 2023

nuclearcat commented Sep 18, 2023

nuclearcat commented Sep 18, 2023

nuclearcat commented Oct 3, 2023

gctucker commented Oct 3, 2023

JenySadadia commented Oct 3, 2023

JenySadadia left a comment

JenySadadia left a comment

JenySadadia commented Oct 3, 2023

gctucker commented Oct 3, 2023

r-c-n commented Oct 3, 2023

nuclearcat commented Oct 3, 2023

JenySadadia commented Oct 3, 2023

base_api.py: Implement operators and document them in cli help #2109

base_api.py: Implement operators and document them in cli help #2109

Conversation

nuclearcat commented Sep 18, 2023 • edited by gctucker Loading

gctucker commented Sep 18, 2023

gctucker commented Sep 18, 2023 • edited Loading

nuclearcat commented Sep 18, 2023

nuclearcat commented Sep 18, 2023

nuclearcat commented Sep 18, 2023

nuclearcat commented Oct 3, 2023

gctucker commented Oct 3, 2023

JenySadadia commented Oct 3, 2023

JenySadadia left a comment

Choose a reason for hiding this comment

JenySadadia left a comment

Choose a reason for hiding this comment

JenySadadia commented Oct 3, 2023

gctucker commented Oct 3, 2023

r-c-n commented Oct 3, 2023

nuclearcat commented Oct 3, 2023

JenySadadia commented Oct 3, 2023

nuclearcat commented Sep 18, 2023 •

edited by gctucker

Loading

gctucker commented Sep 18, 2023 •

edited

Loading