Regression with cliff 2.10.0

Bug #1741911 reported by Julien Danjou
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Magnum
Fix Released
Undecided
yatin
cliff
New
Undecided
Unassigned

Bug Description

commit c6fc228a250aeb22d7079b4abb9f7f19f1cbc814 (HEAD, refs/bisect/bad)
Author: Doug Hellmann <email address hidden>
Date: Wed Nov 22 15:09:18 2017 -0500

    add support for legacy command name translation

    Add add_legacy_command() method to the command manager and update the
    command search logic to try translating the old legacy name to the new
    name.

    Change-Id: I0c0cdbfcb1612ec3975864fc7f730ff186879027
    Signed-off-by: Doug Hellmann <email address hidden>

broke https://github.com/jd/pifpaf

$ pifpaf run mysql -- zsh
pifpaf: 'run mysql -- zsh' is not a pifpaf command. See 'pifpaf --help'.
Did you mean one of these?
  help

Revision history for this message
Javier Peña (jpena-c) wrote :

This is currently breaking the Magnum gate, see [1]. I've been looking at the code, and what I've found is:

Magnum inits DriverCommandManager using None as a namespace parameter [2]. With the cliff change, the _load_commands() method skips any loading, see [3], so this is causing the unit test error, which btw is also seen in the upstream gate.

I'm not sure if setting namespace as None is valid, so this could be fixed on the Magnum side or on the cliff side (if it's expected to be valid). Apparently, using None as a namespace is relatively widespread [4].

[1] -http://logs.openstack.org/30/529330/2/check/openstack-tox-py27/2e898ff/job-output.txt.gz#_2018-01-10_16_13_06_384428
[2] - https://github.com/openstack/magnum/blob/master/magnum/cmd/driver_manage.py#L87-L93
[3] - https://github.com/openstack/cliff/blob/2.10.0/cliff/commandmanager.py#L55-L56
[4] - http://codesearch.openstack.org/?q=CommandManager%5C(None&i=nope&files=&repos=

Revision history for this message
Julien Danjou (jdanjou) wrote :

Yeah, this is how we fixed in pifpaf and gnocchiclient. Just pass a string to DriverCommandManager.

Revision history for this message
Doug Hellmann (doug-hellmann) wrote :

What behavior do you expect when the namespace is set to None? Just that no plugins are loaded?

Changed in magnum:
assignee: nobody → yatin (yatinkarel)
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to magnum (master)

Reviewed: https://review.openstack.org/532741
Committed: https://git.openstack.org/cgit/openstack/magnum/commit/?id=604bbe5f339a1dccf06bc655b9c33ec170587137
Submitter: Zuul
Branch: master

commit 604bbe5f339a1dccf06bc655b9c33ec170587137
Author: yatin <email address hidden>
Date: Thu Jan 11 11:54:56 2018 +0530

    Fix Usage of cliff commandmanager

    Set namespace to "magnum" when setting up cliff
    commandmanager. Earlier None used to work but after
    [1] and it's u-c release as cliff 2.10.0([2]) it requires
    to be set as some string.

    [1] https://review.openstack.org/#/c/522380/
    [2] https://review.openstack.org/#/c/531764/

    Change-Id: I036386d758a9351f053338f41a595fc030df5a33
    Closes-Bug: #1741911

Changed in magnum:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/magnum 6.0.0

This issue was fixed in the openstack/magnum 6.0.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.