Change in vdsm[ovirt-3.5]: tool: Fix error message for non-existing module

Hello Nir Soffer, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/32055

to review the following change.

Change subject: tool: Fix error message for non-existing module
......................................................................

tool: Fix error message for non-existing module

When user specify non existing module (e.g. foo)using the --module
option, he would get this confusing error:

    UsageError: error: argument --module: invalid choice: foo (choose from
['foo'])

While the availabe modules are certificates, libvirt, and sanlock.

Now the error display the available modules, sorted, and formatted in a
user friendly way, instead of as Python list:

    UsageError: error: argument --module: invalid choice: foo
    (available: certificates, libvirt, sanlock)

The relevant test use now real names, so errors look like real errors,
which help to format them properly.

Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1132677
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/31742
Reviewed-by: mooli tayer <mtayer(a)redhat.com&gt;
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M lib/vdsm/tool/configurator.py
M tests/toolTests.py
2 files changed, 6 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/55/32055/1

diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 7f4dc1a..6130d56 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -180,9 +180,10 @@
         try:
             requiredNames = _getConfigurers()[next_].getRequires()
         except KeyError:
+            available = ', '.join(sorted(_getConfigurers()))
             raise UsageError(
-                "error: argument --module: "
-                "invalid choice: %s (choose from %s)\n" % (next_,
modulesNames)
+                "error: argument --module: invalid choice: %s\n"
+                "(available: %s)\n" % (next_, available)
             )
 
         for requiredName in requiredNames:
diff --git a/tests/toolTests.py b/tests/toolTests.py
index b41ae50..88e249f 100644
--- a/tests/toolTests.py
+++ b/tests/toolTests.py
@@ -146,7 +146,8 @@
         configurator,
         '_getConfigurers',
         lambda:  {
-            'a': MockModuleConfigurator('a', set()),
+            'libvirt': MockModuleConfigurator('libvirt', set()),
+            'sanlock': MockModuleConfigurator('sanlock', set()),
         }
     )
     def testNonExistentModule(self):
@@ -155,7 +156,7 @@
             UsageError,
             configurator._parse_args,
             'validate-config',
-            '--module=b'
+            '--module=multipath'
         )
 
 


-- 
To view, visit http://gerrit.ovirt.org/32055
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: tool: Fix error message for non-existing module
......................................................................


Patch Set 2: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/32055
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Yaniv Bronhaim has posted comments on this change.

Change subject: tool: Fix error message for non-existing module
......................................................................


Patch Set 2: Code-Review+2

-- 
To view, visit http://gerrit.ovirt.org/32055
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

mooli tayer has posted comments on this change.

Change subject: tool: Fix error message for non-existing module
......................................................................


Patch Set 1: Verified+1

-- 
To view, visit http://gerrit.ovirt.org/32055
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-HasComments: No

mooli tayer has posted comments on this change.

Change subject: tool: Fix error message for non-existing module
......................................................................


Patch Set 2: Verified+1

-- 
To view, visit http://gerrit.ovirt.org/32055
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-HasComments: No

Yaniv Bronhaim has submitted this change and it was merged.

Change subject: tool: Fix error message for non-existing module
......................................................................


tool: Fix error message for non-existing module

When user specify non existing module (e.g. foo)using the --module
option, he would get this confusing error:

    UsageError: error: argument --module: invalid choice: foo (choose from
['foo'])

While the availabe modules are certificates, libvirt, and sanlock.

Now the error display the available modules, sorted, and formatted in a
user friendly way, instead of as Python list:

    UsageError: error: argument --module: invalid choice: foo
    (available: certificates, libvirt, sanlock)

The relevant test use now real names, so errors look like real errors,
which help to format them properly.

Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1127877
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/31742
Reviewed-by: mooli tayer <mtayer(a)redhat.com&gt;
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
Reviewed-on: http://gerrit.ovirt.org/32055
Reviewed-by: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Tested-by: mooli tayer <mtayer(a)redhat.com&gt;
---
M lib/vdsm/tool/configurator.py
M tests/toolTests.py
2 files changed, 6 insertions(+), 4 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Yaniv Bronhaim: Looks good to me, approved
  mooli tayer: Verified



-- 
To view, visit http://gerrit.ovirt.org/32055
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: tool: Fix error message for non-existing module
......................................................................


Patch Set 3:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms_merged/113/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/32055
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib794b4cb0dcedbbd5b5bb4bb4396860e5edd8710
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No