← Back to team overview

canonical-ubuntu-qa team mailing list archive

Re: [Merge] ~hyask/autopkgtest-cloud:skia/explicit_reject_msg into autopkgtest-cloud:master

 


Diff comments:

> diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
> index 1316336..eaabc36 100755
> --- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
> +++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
> @@ -729,6 +730,7 @@ def request(msg):
>              body = msg.body.decode("UTF-8")
>          except UnicodeDecodeError as e:
>              logging.error('Bad encoding in request "%s": %s', msg.body, e)
> +            msg.channel.basic_ack(msg.delivery_tag)

what's the choice for ack instead of basic_reject here? As I understand it, fundamentally they do the same thing with `requeue=False`, but perhaps it'd help for readability here if it was also basic_reject with `requeue=False`?

>              return
>  
>      # request is either a single string pkgname or "pkg_name json_params"
> @@ -743,6 +745,7 @@ def request(msg):
>          logging.error(
>              'Received invalid request format "%s" (%s)', body, repr(e)
>          )
> +        msg.channel.basic_ack(msg.delivery_tag)

same applies as above

>          return
>      test_uuid = params.get("uuid", str(uuid.uuid4()))
>      if not re.match("[a-zA-Z0-9.+-]+$", pkgname):


-- 
https://code.launchpad.net/~hyask/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/465143
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~hyask/autopkgtest-cloud:skia/explicit_reject_msg into autopkgtest-cloud:master.



References