Juan Hernandez has uploaded a new change for review.

Change subject: cli: Don't depend on optparse
......................................................................

cli: Don't depend on optparse

Currently the dependency on the "optparse" module is deep in the
implementation of the CLI. This causes issues, like unexpected exit from
the "connect" command when passing incorrect options. To avoid these
issues this patch isolates the use of the "optparse" module to the main
program, so that the rest of the code will just receive the already
parsed options.

Change-Id: If3d71de72f929125229e8c8d0e86df5239fad07d
Bug-Url: https://bugzilla.redhat.com/1135550
Signed-off-by: Juan Hernandez <[email protected]>
---
M src/cli/context.py
M src/ovirtcli/infrastructure/context.py
M src/ovirtcli/infrastructure/settings.py
M src/ovirtcli/main.py
M src/ovirtcli/shell/actioncmdshell.py
M src/ovirtcli/shell/addcmdshell.py
M src/ovirtcli/shell/capabilitiescmdshell.py
M src/ovirtcli/shell/clearcmdshell.py
M src/ovirtcli/shell/cmdshell.py
M src/ovirtcli/shell/connectcmdshell.py
M src/ovirtcli/shell/consolecmdshell.py
M src/ovirtcli/shell/disconnectcmdshell.py
M src/ovirtcli/shell/engineshell.py
M src/ovirtcli/shell/filecmdshell.py
M src/ovirtcli/shell/historycmdshell.py
M src/ovirtcli/shell/infocmdshell.py
M src/ovirtcli/shell/listcmdshell.py
M src/ovirtcli/shell/pingcmdshell.py
M src/ovirtcli/shell/removecmdshell.py
M src/ovirtcli/shell/showcmdshell.py
M src/ovirtcli/shell/statuscmdshell.py
M src/ovirtcli/shell/summarycmdshell.py
M src/ovirtcli/shell/updatecmdshell.py
23 files changed, 99 insertions(+), 111 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine-cli refs/changes/57/36257/1

diff --git a/src/cli/context.py b/src/cli/context.py
index 35472e8..a322b85 100644
--- a/src/cli/context.py
+++ b/src/cli/context.py
@@ -57,7 +57,7 @@
     welcome = None
     goodbye = None
 
-    def __init__(self, cmdin=None, args=None):
+    def __init__(self, cmdin=None, args=None, opts=None):
         """Constructor."""
         self.parameters_cash = {}
         self.cmdin = cmdin or sys.stdin
@@ -76,6 +76,23 @@
         self.__setup_pager()
         self.__encoding = None
 
+        # Copy the command line options into the settings:
+        if opts is not None:
+            for name, value in opts.iteritems():
+                if value is None:
+                    continue
+                name = name.replace('-', '_')
+                if name in ('debug', 'verbosity'):
+                    key = 'cli:%s' % name
+                else:
+                    key = 'ovirt-shell:%s' % name
+                try:
+                    self.settings[key] = value
+                except KeyError:
+                    pass
+                except ValueError, e:
+                    sys.stderr.write('error: %s\n' % str(e))
+
     def _setup_logging(self):
         """Configure logging."""
         logger = logging.getLogger()
diff --git a/src/ovirtcli/infrastructure/context.py 
b/src/ovirtcli/infrastructure/context.py
index 1221c26..2de56aa 100644
--- a/src/ovirtcli/infrastructure/context.py
+++ b/src/ovirtcli/infrastructure/context.py
@@ -43,8 +43,8 @@
     CLI_MODULE_NAME = "ovirt-shell"
     DEFAULT_VERSION = (0, 0, 0, 0)
 
-    def __init__(self, args=None):
-        super(OvirtCliExecutionContext, self).__init__(args=args)
+    def __init__(self, args=None, opts=None):
+        super(OvirtCliExecutionContext, self).__init__(args=args, opts=opts)
         self.connection = None
         self.url = None
         self.formatter = create(Formatter, 
self.settings['ovirt-shell:output_format'])
diff --git a/src/ovirtcli/infrastructure/settings.py 
b/src/ovirtcli/infrastructure/settings.py
index ec29ad9..9c2ee4b 100644
--- a/src/ovirtcli/infrastructure/settings.py
+++ b/src/ovirtcli/infrastructure/settings.py
@@ -62,6 +62,7 @@
         ('ovirt-shell:prompt', str, ''),
         ('ovirt-shell:extended_prompt', boolean, False),
         ('ovirt-shell:execute_command', str, None),
+        ('ovirt-shell:file', str, None),
         ('ovirt-shell:renew_session', boolean, False),
         ('ovirt-shell:kerberos', boolean, False),
     ]
diff --git a/src/ovirtcli/main.py b/src/ovirtcli/main.py
index 39149e4..ac7d5ce 100644
--- a/src/ovirtcli/main.py
+++ b/src/ovirtcli/main.py
@@ -21,24 +21,21 @@
 from ovirtcli.infrastructure.object import create
 from ovirtcli.shell.engineshell import EngineShell
 
-    ############################## MAIN #################################
 def main():
+    # Parse the command line:
     parser = create(OvirtCliOptionParser)
-    context = OvirtCliExecutionContext(sys.argv)
-    shell = EngineShell(context, parser)
+    opts, args = parser.parse_args()
 
-    if len(sys.argv) > 1:
-        args = ''
-        if len(sys.argv) > 2:
-            args = sys.argv[1] + " "
-            for item in sys.argv[2:]:
-                args += '"' + item + '" '
-        else:
-            args = ' '.join(sys.argv[1:])
-        shell.onecmd_loop(args)
-    else:
-        shell.onecmd_loop('')
-    ########################### __main__ #################################
+    # Convert the options to a dictionary, so the rest of the code doesn't
+    # have to deal with optparse specifics:
+    opts = vars(opts)
+
+    # Create the execution context:
+    context = OvirtCliExecutionContext(opts=opts, args=args)
+
+    # Create the command interpreter:
+    shell = EngineShell(context)
+    shell.onecmd_loop()
+
 if __name__ == '__main__':
     main()
-    ######################################################################
diff --git a/src/ovirtcli/shell/actioncmdshell.py 
b/src/ovirtcli/shell/actioncmdshell.py
index 93e03b5..78c4317 100644
--- a/src/ovirtcli/shell/actioncmdshell.py
+++ b/src/ovirtcli/shell/actioncmdshell.py
@@ -25,8 +25,8 @@
 class ActionCmdShell(CmdShell):
     NAME = 'action'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
         self.complete_exceptions = ['delete', 'update']
         self.identifier_template = '--%s-identifier'
 
diff --git a/src/ovirtcli/shell/addcmdshell.py 
b/src/ovirtcli/shell/addcmdshell.py
index 8398581..0c26a40 100644
--- a/src/ovirtcli/shell/addcmdshell.py
+++ b/src/ovirtcli/shell/addcmdshell.py
@@ -25,8 +25,8 @@
 class AddCmdShell(CmdShell):
     NAME = 'add'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_add(self, args):
         return self.context.execute_string(AddCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/capabilitiescmdshell.py 
b/src/ovirtcli/shell/capabilitiescmdshell.py
index 7ff899a..64d7d22 100644
--- a/src/ovirtcli/shell/capabilitiescmdshell.py
+++ b/src/ovirtcli/shell/capabilitiescmdshell.py
@@ -25,8 +25,8 @@
        'features'
     ]
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_capabilities(self, args):
         return self.context.execute_string(CapabilitiesCmdShell.NAME + ' ' + 
args + '\n')
diff --git a/src/ovirtcli/shell/clearcmdshell.py 
b/src/ovirtcli/shell/clearcmdshell.py
index e11d07f..bb31587 100644
--- a/src/ovirtcli/shell/clearcmdshell.py
+++ b/src/ovirtcli/shell/clearcmdshell.py
@@ -21,8 +21,8 @@
 class ClearCmdShell(CmdShell):
     NAME = 'clear'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_clear(self, args):
         self.context.execute_string(ClearCmdShell.NAME + ' ' + args + '\n')
diff --git a/src/ovirtcli/shell/cmdshell.py b/src/ovirtcli/shell/cmdshell.py
index 794c700..349e840 100644
--- a/src/ovirtcli/shell/cmdshell.py
+++ b/src/ovirtcli/shell/cmdshell.py
@@ -23,23 +23,17 @@
 
 
 class CmdShell(object):
-    def __init__(self, context, parser):
+    def __init__(self, context):
         self.__context = context
-        self.__parser = parser
         self.__owner = self
 
     def get_owner(self):
         return self.__owner
 
-
-    def get_parser(self):
-        return self.__parser
-
     def get_context(self):
         return self.__context
 
     context = property(get_context, None, None, None)
-    parser = property(get_parser, None, None, None)
     owner = property(get_owner, None, None, None)
 
     ############################ CONFIG #################################
diff --git a/src/ovirtcli/shell/connectcmdshell.py 
b/src/ovirtcli/shell/connectcmdshell.py
index 8db81f3..64fac89 100644
--- a/src/ovirtcli/shell/connectcmdshell.py
+++ b/src/ovirtcli/shell/connectcmdshell.py
@@ -14,12 +14,9 @@
 # limitations under the License.
 #
 
-
-import shlex
-import sys
-
 from ovirtcli.shell.cmdshell import CmdShell
 from ovirtcli.utils.autocompletionhelper import AutoCompletionHelper
+
 
 class ConnectCmdShell(CmdShell):
     NAME = 'connect'
@@ -38,28 +35,11 @@
        'kerberos',
     ]
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
-
-    def __do_verbose_connect(self, context, parser, opts):
-        if not self.copy_environment_vars(context):
-            sys.exit(1)
-        if not self.copy_cmdline_options(opts, context, parser):
-            sys.exit(1)
-        self.context.execute_string(ConnectCmdShell.NAME + '\n')
-
-    def __do_connect(self, args):
-        arg = ConnectCmdShell.NAME + ' ' + args.strip() + '\n'
-        self.context.execute_string(arg)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_connect(self, args):
-        arg = '--connect ' + args.strip() + '\n'
-        m_opts, m_args = self.parser.parse_args(args=shlex.split(arg))
-
-        if m_opts.connect and len(m_args) == 0:
-            self.__do_verbose_connect(self.context, self.parser, m_opts)
-        else:
-            self.__do_connect(args)
+        self.context.execute_string(ConnectCmdShell.NAME + ' ' + args + '\n')
 
     def complete_connect(self, text, line, begidx, endidx):
         return AutoCompletionHelper.complete(line=line, text=text,
diff --git a/src/ovirtcli/shell/consolecmdshell.py 
b/src/ovirtcli/shell/consolecmdshell.py
index 1710534..7edc61b 100644
--- a/src/ovirtcli/shell/consolecmdshell.py
+++ b/src/ovirtcli/shell/consolecmdshell.py
@@ -21,8 +21,8 @@
 class ConsoleCmdShell(CmdShell):
     NAME = 'console'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_console(self, args):
         return self.context.execute_string(ConsoleCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/disconnectcmdshell.py 
b/src/ovirtcli/shell/disconnectcmdshell.py
index 9d6900f..4f02b3b 100644
--- a/src/ovirtcli/shell/disconnectcmdshell.py
+++ b/src/ovirtcli/shell/disconnectcmdshell.py
@@ -21,8 +21,8 @@
 class DisconnectCmdShell(CmdShell):
     NAME = 'disconnect'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_disconnect(self, args):
         return self.context.execute_string(DisconnectCmdShell.NAME + ' ' + 
args + '\n')
diff --git a/src/ovirtcli/shell/engineshell.py 
b/src/ovirtcli/shell/engineshell.py
index 03876c7..db9fe4b 100644
--- a/src/ovirtcli/shell/engineshell.py
+++ b/src/ovirtcli/shell/engineshell.py
@@ -63,25 +63,25 @@
 
     ############################# INIT #################################
 
-    def __init__(self, context, parser, completekey='tab', stdin=None, 
stdout=None):
+    def __init__(self, context, completekey='tab', stdin=None, stdout=None):
         cmd.Cmd.__init__(self, completekey=completekey, stdin=stdin, 
stdout=stdout)
-        ConnectCmdShell.__init__(self, context, parser)
-        ActionCmdShell.__init__(self, context, parser)
-        ShowCmdShell.__init__(self, context, parser)
-        ListCmdShell.__init__(self, context, parser)
-        UpdateCmdShell.__init__(self, context, parser)
-        RemoveCmdShell.__init__(self, context, parser)
-        AddCmdShell.__init__(self, context, parser)
-        DisconnectCmdShell.__init__(self, context, parser)
-        ConsoleCmdShell.__init__(self, context, parser)
-        PingCmdShell.__init__(self, context, parser)
-        StatusCmdShell.__init__(self, context, parser)
-        ClearCmdShell.__init__(self, context, parser)
-        FileCmdShell.__init__(self, context, parser)
-        HistoryCmdShell.__init__(self, context, parser)
-        InfoCmdShell.__init__(self, context, parser)
-        SummaryCmdShell.__init__(self, context, parser)
-        CapabilitiesCmdShell.__init__(self, context, parser)
+        ConnectCmdShell.__init__(self, context)
+        ActionCmdShell.__init__(self, context)
+        ShowCmdShell.__init__(self, context)
+        ListCmdShell.__init__(self, context)
+        UpdateCmdShell.__init__(self, context)
+        RemoveCmdShell.__init__(self, context)
+        AddCmdShell.__init__(self, context)
+        DisconnectCmdShell.__init__(self, context)
+        ConsoleCmdShell.__init__(self, context)
+        PingCmdShell.__init__(self, context)
+        StatusCmdShell.__init__(self, context)
+        ClearCmdShell.__init__(self, context)
+        FileCmdShell.__init__(self, context)
+        HistoryCmdShell.__init__(self, context)
+        InfoCmdShell.__init__(self, context)
+        SummaryCmdShell.__init__(self, context)
+        CapabilitiesCmdShell.__init__(self, context)
 
         self.__last_output = ''
         self.__input_buffer = ''
@@ -329,20 +329,19 @@
     def _get_last_status(self):
         return self.__last_status
 
-    def onecmd_loop(self, s):
-        opts, args = self.parser.parse_args()
-        del args
-        self.__persist_cmd_options(opts)
-        if opts.connect or self.context.settings.get('cli:autoconnect'):
+    def onecmd_loop(self):
+        if self.context.settings.get('cli:autoconnect'):
             self.context._collect_connection_data()
-            if not self.context.settings.get('ovirt-shell:execute_command'):
+            file = self.context.settings.get('ovirt-shell:file')
+            execute_command = 
self.context.settings.get('ovirt-shell:execute_command')
+            if not execute_command:
                 self.do_clear('')
-            self.do_connect(s)
-            if opts.file:
-                self.do_file(opts.file)
+            self.do_connect('')
+            if file:
+                self.do_file(file)
                 self.do_exit('')
-            elif opts.execute_command:
-                self.__execute_command(opts.execute_command)
+            elif execute_command:
+                self.__execute_command(execute_command)
                 self.do_exit('')
             else:
                 self.cmdloop(clear=False)
diff --git a/src/ovirtcli/shell/filecmdshell.py 
b/src/ovirtcli/shell/filecmdshell.py
index 89dc4df..65eb302 100644
--- a/src/ovirtcli/shell/filecmdshell.py
+++ b/src/ovirtcli/shell/filecmdshell.py
@@ -21,8 +21,8 @@
 class FileCmdShell(CmdShell):
     NAME = 'file'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_file(self, arg):
         """\
diff --git a/src/ovirtcli/shell/historycmdshell.py 
b/src/ovirtcli/shell/historycmdshell.py
index 7dc181e..76f184c 100644
--- a/src/ovirtcli/shell/historycmdshell.py
+++ b/src/ovirtcli/shell/historycmdshell.py
@@ -25,8 +25,8 @@
        'last', 'first'
     ]
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_history(self, args):
         return self.context.execute_string(HistoryCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/infocmdshell.py 
b/src/ovirtcli/shell/infocmdshell.py
index 1afb8c9..87521f5 100644
--- a/src/ovirtcli/shell/infocmdshell.py
+++ b/src/ovirtcli/shell/infocmdshell.py
@@ -21,8 +21,8 @@
 class InfoCmdShell(CmdShell):
     NAME = 'info'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_info(self, args):
         return self.context.execute_string(InfoCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/listcmdshell.py 
b/src/ovirtcli/shell/listcmdshell.py
index a07759d..36cad99 100644
--- a/src/ovirtcli/shell/listcmdshell.py
+++ b/src/ovirtcli/shell/listcmdshell.py
@@ -26,8 +26,8 @@
     NAME = 'list'
     OPTIONS = ['show-all']
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_list(self, args):
         return self.context.execute_string(ListCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/pingcmdshell.py 
b/src/ovirtcli/shell/pingcmdshell.py
index 0291df4..272533c 100644
--- a/src/ovirtcli/shell/pingcmdshell.py
+++ b/src/ovirtcli/shell/pingcmdshell.py
@@ -21,8 +21,8 @@
 class PingCmdShell(CmdShell):
     NAME = 'ping'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_ping(self, args):
         return self.context.execute_string(PingCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/removecmdshell.py 
b/src/ovirtcli/shell/removecmdshell.py
index 52c7335..6d07860 100644
--- a/src/ovirtcli/shell/removecmdshell.py
+++ b/src/ovirtcli/shell/removecmdshell.py
@@ -26,8 +26,8 @@
     NAME = 'remove'
     ALIAS = 'delete'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_remove(self, args):
         return self.context.execute_string(RemoveCmdShell.ALIAS + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/showcmdshell.py 
b/src/ovirtcli/shell/showcmdshell.py
index a3a48b1..b829d6a 100644
--- a/src/ovirtcli/shell/showcmdshell.py
+++ b/src/ovirtcli/shell/showcmdshell.py
@@ -26,8 +26,8 @@
     NAME = 'show'
     ALIAS = 'get'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_show(self, args):
         return self.context.execute_string(ShowCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/statuscmdshell.py 
b/src/ovirtcli/shell/statuscmdshell.py
index b997bde..ea4a327 100644
--- a/src/ovirtcli/shell/statuscmdshell.py
+++ b/src/ovirtcli/shell/statuscmdshell.py
@@ -21,8 +21,8 @@
 class StatusCmdShell(CmdShell):
     NAME = 'status'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_status(self, args):
         return self.context.execute_string(StatusCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/summarycmdshell.py 
b/src/ovirtcli/shell/summarycmdshell.py
index a22b5c5..8863b86 100644
--- a/src/ovirtcli/shell/summarycmdshell.py
+++ b/src/ovirtcli/shell/summarycmdshell.py
@@ -21,8 +21,8 @@
 class SummaryCmdShell(CmdShell):
     NAME = 'summary'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_summary(self, args):
         return self.context.execute_string(SummaryCmdShell.NAME + ' ' + args + 
'\n')
diff --git a/src/ovirtcli/shell/updatecmdshell.py 
b/src/ovirtcli/shell/updatecmdshell.py
index 9ce4969..7c59b69 100644
--- a/src/ovirtcli/shell/updatecmdshell.py
+++ b/src/ovirtcli/shell/updatecmdshell.py
@@ -25,8 +25,8 @@
 class UpdateCmdShell(CmdShell):
     NAME = 'update'
 
-    def __init__(self, context, parser):
-        CmdShell.__init__(self, context, parser)
+    def __init__(self, context):
+        CmdShell.__init__(self, context)
 
     def do_update(self, args):
         return self.context.execute_string(UpdateCmdShell.NAME + ' ' + args + 
'\n')


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If3d71de72f929125229e8c8d0e86df5239fad07d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine-cli
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to