Page MenuHomePhabricator
Create Task
Maniphest T249390

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

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.

Details

SubjectRepoBranchLines +/-
d/changelog: prepare for 0.66 releaseoperations/software/tools-webservicemaster+10 -0
toolforge: ensure the python2 backport of configparser is installedoperations/puppetproduction+2 -0
args: A few fixupsoperations/software/tools-webservicemaster+6 -11
Fix partial rename of "type" parameter to "wstype"operations/software/tools-webservicemaster+37 -33
Customize query in gerrit

Related Objects
Search...

Event Timeline

• Bstorm triaged this task as High priority.Apr 3 2020, 11:15 PM
• Bstorm created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 3 2020, 11:15 PM
• Bstorm added parent tasks: T234617: Toolforge. introduce new domain toolforge.org, T154504: Make webservice backend default to kubernetes.Apr 3 2020, 11:17 PM
• Bstorm added a project: Toolforge.
gerritbot added a comment.Apr 4 2020, 12:15 AM

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

gerritbot added a project: Patch-For-Review.Apr 4 2020, 12:15 AM
• bd808 moved this task from Backlog to Waiting for code review on the Toolforge board.Apr 6 2020, 4:59 AM
• bd808 moved this task from Inbox to Doing on the cloud-services-team (Kanban) board.Apr 6 2020, 5:03 AM
gerritbot added a comment.Apr 7 2020, 9:49 PM

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

• Bstorm added a comment.Apr 7 2020, 9:53 PM

Ok, that's one down!

Maintenance_bot removed a project: Patch-For-Review.Apr 7 2020, 10:10 PM
• Bstorm added a comment.Apr 7 2020, 10:40 PM

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.

• Bstorm added a comment.Apr 7 2020, 11:03 PM

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.

• Bstorm added a comment.Apr 7 2020, 11:07 PM

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.

gerritbot added a comment.Apr 7 2020, 11:16 PM

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

gerritbot added a project: Patch-For-Review.Apr 7 2020, 11:16 PM
gerritbot added a comment.Apr 7 2020, 11:35 PM

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

gerritbot added a comment.Apr 8 2020, 12:38 AM

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

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

gerritbot added a comment.Apr 8 2020, 12:44 AM

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

gerritbot added a comment.Apr 8 2020, 12:53 AM

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

gerritbot added a comment.Apr 8 2020, 1:06 AM

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

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

Stashbot added a comment.Apr 8 2020, 1:10 AM

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

Maintenance_bot removed a project: Patch-For-Review.Apr 8 2020, 1:10 AM
• Bstorm closed this task as Resolved.Apr 8 2020, 11:36 PM
• Bstorm claimed this task.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits