Page MenuHomePhabricator

Regression in webservice 0.65 in argument and config parsing
Closed, ResolvedPublic

Description

In doing some QA on webservice 0.65, there's a couple odd behaviors that are new.

  1. The parsing of arguments is off somehow. webservice python3.7 start --backend kubernetes --cpu 1 --mem 1Gi is treated as a gridengine service. That seems wrong on several levels since k8s is the default backend.
  2. When it got things right using webservice --backend kubernetes --cpu 1 --mem 1Gi python3.7 start, it crashed with:
Traceback (most recent call last):
  File "/usr/local/bin/webservice", line 442, in <module>
    extra_args=args.extra_args,
  File "/usr/lib/python2.7/dist-packages/toolforge/webservice/backends/kubernetesbackend.py", line 173, in __init__
    self.webservice = KubernetesBackend.CONFIG[type]["cls"](
KeyError: <type 'type'>

So this version is not ready for release.

Event Timeline

Bstorm triaged this task as High priority.Apr 3 2020, 11:15 PM
Bstorm created this task.

Change 585846 had a related patch set uploaded (by BryanDavis; owner: Bryan Davis):
[operations/software/tools-webservice@master] Fix partial rename of "type" parameter to "wstype"

https://gerrit.wikimedia.org/r/585846

Change 585846 merged by jenkins-bot:
[operations/software/tools-webservice@master] Fix partial rename of "type" parameter to "wstype"

https://gerrit.wikimedia.org/r/585846

Found one reason my testing was producing a grid service.
There's a .webservicerc file in the test tool that is set to gridengine!!! At very least, now it makes sense that it defaults to grid.

Found the issue with #1. Because extra_args is positional, it eats everything after the other two positionals.

I proved it by "print"ing extra_args in my qa folder:

./webservice python3.7 start --backend kubernetes --cpu 1 --mem 1Gi
['--backend', 'kubernetes', '--cpu', '1', '--mem', '1Gi']

This makes perfect, obnoxious, sense. I think it might bear documenting in the help message. I had selected gridengine as the backend because of a .webservicerc, and we eat everything after the action as extra_args whether you like it or not. So we are forcing a stricter order on args than was previously allowed. Looks like it is the use of argparse.REMAINDER that did it.

Yes that did it. Switching extra_args back to "*" for nargs makes the example in #1 work fine. Using argparse.REMAINDER makes it all into extra_args.

Change 587369 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[operations/software/tools-webservice@master] args: A few fixups

https://gerrit.wikimedia.org/r/587369

Change 587372 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[operations/puppet@production] toolforge: ensure the python2 backport of configparser is installed

https://gerrit.wikimedia.org/r/587372

Change 587369 merged by jenkins-bot:
[operations/software/tools-webservice@master] args: A few fixups

https://gerrit.wikimedia.org/r/587369

Change 587372 abandoned by Bstorm:
toolforge: ensure the python2 backport of configparser is installed

Reason:
Added it to the package depends instead

https://gerrit.wikimedia.org/r/587372

Change 587383 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[operations/software/tools-webservice@master] d/changelog: prepare for 0.66 release

https://gerrit.wikimedia.org/r/587383

Change 587383 merged by jenkins-bot:
[operations/software/tools-webservice@master] d/changelog: prepare for 0.66 release

https://gerrit.wikimedia.org/r/587383

Mentioned in SAL (#wikimedia-cloud) [2020-04-08T01:10:02Z] <bstorm_> upgrade toollabs-webservice to 0.66 for qa T249390

Bstorm claimed this task.