#159 Add a --get parameter to search if there are waivers
Closed 2 years ago by ralph. Opened 2 years ago by pingou.
pingou/waiverdb get_waivers  into  master

file modified
+13 -5
@@ -84,15 +84,22 @@ 

  ```

  Usage: waiverdb-cli [OPTIONS]

  

-   Creates new waivers against test results.

+   Creates new waiver or retrieve recorded waivers against test results

  

-   Examples:

+   Examples to set a waiver:

  

        waiverdb-cli -r 47 -r 48 -p "fedora-28" -c "This is fine"

- or

  

-       waiverdb-cli -t dist.rpmdeplint -s '{"item": "qclib-1.3.1-3.fc28", "type": "koji_build"}' -p "fedora-28" -c "This is expected for non-x86 packages"

+       or

  

+       waiverdb-cli -t dist.rpmdeplint -s '{"item": "qclib-1.3.1-3.fc28",

+       "type": "koji_build"}' -p "fedora-28" -c "This is expected for non-x86

+       packages"

+ 

+   Example to retrieve waivers:

+ 

+       waiverdb-cli --get -t dist.rpmdeplint -s '{"item":

+       "qclib-1.3.1-3.fc28", "type": "koji_build"}' -p fedora-28

  

  Options:

    -C, --config-file PATH      Specify a config file to use
@@ -101,7 +108,8 @@ 

    -t, --testcase TEXT         Specify a testcase for the subject

    -p, --product-version TEXT  Specify one of PDC's product version

                                identifiers.

-   --waived / --no-waived      Whether or not the result is waived

+   --waived / --no-waived      Waive or unwaive the result, defaults to "waive"

+   --get                       Search if the result has been waived

    -c, --comment TEXT          A comment explaining why the result is waived

    -h, --help                  Show this message and exit.

  ```

file modified
+33 -7
@@ -66,22 +66,27 @@ 

  @click.option('--product-version', '-p',

                help='Specify one of PDC\'s product version identifiers.')

  @click.option('--waived/--no-waived', default=True,

-               help='Whether or not the result is waived')

+               help='Waive or unwaive the result, defaults to "waive"')

+ @click.option('--get', default=False, is_flag=True,

+               help='Search if the result has been waived')

  @click.option('--comment', '-c',

                help='A comment explaining why the result is waived')

- def cli(comment, waived, product_version, testcase, subject, result_id, config_file):

+ def cli(comment, get, waived, product_version, testcase, subject,

+         result_id, config_file):

      """

-     Creates new waiver against test results.

+     Creates new waiver or retrieve recorded waivers against test results

  

-     Examples:

+     Examples to set a waiver:

  

          waiverdb-cli -r 47 -r 48 -p "fedora-28" -c "This is fine"

  

          or

  

-         waiverdb-cli -t dist.rpmdeplint -s '{"item": "qclib-1.3.1-3.fc28",

-                                              "type": "koji_build"}'

-                      -p "fedora-28" -c "This is expected for non-x86 packages"

+         waiverdb-cli -t dist.rpmdeplint -s '{"item": "qclib-1.3.1-3.fc28", "type": "koji_build"}' -p "fedora-28" -c "This is expected for non-x86 packages"

+ 

+     Example to retrieve waivers:

+ 

+         waiverdb-cli --get -t dist.rpmdeplint -s '{"item": "qclib-1.3.1-3.fc28", "type": "koji_build"}' -p fedora-28

  

      """

      config = configparser.SafeConfigParser()
@@ -120,6 +125,27 @@ 

          })

  

      api_url = config.get('waiverdb', 'api_url')

+     if get:

+         req = requests.get(

+             url='{0}/waivers/'.format(api_url.rstrip('/')),

+             params={'results': json.dumps(data_list)},

+             timeout=60,

+         )

+         if req.ok:

+             data = req.json()

+             cnt_waiver = len(data['data'])

+             if cnt_waiver == 1:

+                 print('{0} waiver found'.format(cnt_waiver))

+             else:

+                 print('{0} waiver(s) found'.format(cnt_waiver))

+ 

+             for waiver in data['data']:

+                 print('Waiver from {0}, created on {1} for: {2}'.format(

+                     waiver['username'], waiver['timestamp'],

+                     waiver['comment']))

"Comment" can be "None" and in that case it will print something like:
Waiver from user, created on 2018-04-18T13:21:54.842490 for: None

So maybe we can print the comment only if is not None.

Hmm, I thought we enforced that the comment was non-empty on the server side?

No, we don't...
Example:

waiverdb-cli -t dist.rpmdeplint -s '{"item": "qclib-1.3.1-3.fc28", "type": "koji_build"}' -p "fedora-28"
Created waiver 20 for result with subject {"item": "qclib-1.3.1-3.fc28", "type": "koji_build"} and testcase dist.rpmdeplint

Also in the API. We can change it.

edit: https://pagure.io/waiverdb/pull-request/167

+ 

+         return 0

+ 

      if auth_method == 'OIDC':

          # Try to import this now so the user gets immediate feedback if

          # it isn't installed

This is far from ideal, the current implementation basically does not allow
having multiple actions on the same script, we cannot do things like:
waiverdb-cli new <waivers>
waiverdb-cli get <waivers>
waiverdb-cli delete <waiver>
...
without breaking backward compatibility.

So in order to get the functionality for retrieving waivers, I have added a
--get parameter which allows, using the same syntax to either set or get
waivers against the specified subject.
It's not ideal, but from my testing it does work as expected.

We may consider refactoring this one day, but it is not a priority for the
moment.

Fixes https://pagure.io/waiverdb/issue/152

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

@gnaponie, can you provide review on this one?

@ralph yes, I'll put some comment before the end of the day

"Comment" can be "None" and in that case it will print something like:
Waiver from user, created on 2018-04-18T13:21:54.842490 for: None

So maybe we can print the comment only if is not None.

Can you put some test? Something similar to this:
https://paste.fedoraproject.org/paste/YDoUvwRbs5ef-qLfneL8Qw

It would be cool also a test that creates a waiver and then checks that the waiver-cli --get is returning the "1 waiver found" or so.

I think we could add subcommands create and list, and then for backwards compatibility if no subcommand is given, treat it like create. If someone wants to tackle a patch for that :-) There is no ambiguity in the parsing because it doesn't currently accept any positional arguments.

Hmm, I thought we enforced that the comment was non-empty on the server side?

@dcallagh sounds nice, let's see if we can do this with click though :s

No, we don't...
Example:

waiverdb-cli -t dist.rpmdeplint -s '{"item": "qclib-1.3.1-3.fc28", "type": "koji_build"}' -p "fedora-28"
Created waiver 20 for result with subject {"item": "qclib-1.3.1-3.fc28", "type": "koji_build"} and testcase dist.rpmdeplint

Also in the API. We can change it.

edit: https://pagure.io/waiverdb/pull-request/167

How open would you be in dropping click for basic argparse? If so I'll see if I can get argparse to work, because so far not much luck with click

I like Click, but if argparse gets the job done then it's fine too.

@pingou, I'm going to close this one for now to get it out of the review queue. Can you resubmit when you have cycles to circle back on it?

Pull-Request has been closed by ralph

2 years ago

Sure will do, note that the current PR was working afaik, the rework is about getting it in a better shape only.

Build 0bf26a56be48323c4f2c6c15b02793eabc9c98da FAILED!
Rebase or make new commits to rebuild.