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

mtayer at redhat.com mtayer at redhat.com
Wed Aug 27 15:25:10 UTC 2014


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 at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31742
Reviewed-by: mooli tayer <mtayer at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
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 at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list