[PATCH 1/4] Rework copr-cli

[Pierre-Yves Chibon]

This commit restructure copr-cli by removing cliff from it and thus making it a simple
argparse based python program.

This removes complexicity as well as the dependency on cliff.
---

Ok, so here is a series of patch that remove the dependency to cliff, move the
url of the corp into the config file and do some cleaning.

Comments welcome on the part that removes the dependency on cliff.

Thanks,

Pierre

copr_cli/main.py        | 153 +++++++++++++++++++++++++-------------
 copr_cli/subcommands.py | 193 ++++++++++++++++++------------------------------
 coprcli-setup.py        |  27 ++-----
 3 files changed, 179 insertions(+), 194 deletions(-)

diff --git a/copr_cli/main.py b/copr_cli/main.py
index a0c7a8a..a06e5b3 100644
--- a/copr_cli/main.py
+++ b/copr_cli/main.py
@@ -1,69 +1,120 @@
 #!/usr/bin/python -tt
 #-*- coding: UTF-8 -*-

-import logging
+import argparse
 import sys

-
-import cliff.app
-import cliff.commandmanager
-from cliff.commandmanager import CommandManager
+import subcommands

__version__ = '0.1.0'
 __description__ = "CLI tool to run copr"

-copr_url = 'http://copr-fe.cloud.fedoraproject.org/'
-copr_api_url = '{0}/api'.format(copr_url)
-
-
-class CoprCli(cliff.app.App):
-
-    log = logging.getLogger(__name__)
-
-    def __init__(self):
-        manager = cliff.commandmanager.CommandManager('copr_cli.subcommands')
-        super(CoprCli, self).__init__(
-            description=__description__,
-            version=__version__,
-            command_manager=manager,
-        )
-        requests_log = logging.getLogger("requests")
-        requests_log.setLevel(logging.WARN)
-
-    def initialize_app(self, argv):
-        self.log.debug('initialize_app')
-
-    def prepare_to_run_command(self, cmd):
-        self.log.debug('prepare_to_run_command %s', cmd.__class__.__name__)
-
-    def clean_up(self, cmd, result, err):
-        self.log.debug('clean_up %s', cmd.__class__.__name__)
-        if err:
-            self.log.debug('got an error: %s', err)

-    # Overload run_subcommand to gracefully handle unknown commands.
-    def run_subcommand(self, argv):
-        try:
-            self.command_manager.find_command(argv)
-        except ValueError as e:
-            if "Unknown command" in str(e):
-                print "%r is an unknown command" % ' '.join(argv)
-                print "Try "copr -h""
-                sys.exit(1)
-            else:
-                raise
+COPR_URL = 'http://copr-fe.cloud.fedoraproject.org/'
+COPR_API_URL = '{0}/api'.format(COPR_URL)
+
+CMDLIST = ['create', 'build', 'list', 'info']
+
+
+def setup_parser():
+    """
+    Set the main arguments.
+    """
+    usage = "\nCommands: {0}".format(', '.join(CMDLIST))
+    parser = argparse.ArgumentParser(
+    usage="%(prog)s [global options] COMMAND [options]" + usage,
+    prog="copr-cli")
+    # General connection options
+    parser.add_argument('command', help='The type of action you '
+                'would like to perform. Action must be one of %s. '
+                'Run copr-cli <action> -h to get the help of this '
+                'specific action.' %
+                ', '.join(CMDLIST))
+    parser.add_argument('argument', nargs=argparse.REMAINDER)
+    parser.add_argument('--version', action='version',
+                version='copr-cli %s' % (__version__))
+    return parser
+
+
+def setup_action_parser(action, last_args=None):
+    """
+    Parse the remaining argument for action specific arguments.
+
+    :arg action the action for which the rest of the arguments will be
+    parsed. Actions can be "acl" or "list".
+    """
+    parser = argparse.ArgumentParser(
+                usage="%(prog)s {0} [options]".format(action))
+
+    if action == 'Create':
+        parser.add_argument('name', help='The name of the copr to create')
+        parser.add_argument("--chroot", dest="chroots", action='append',
+                            help="")
+        parser.add_argument('--repo', dest='repos', action='append',
+                            help="")
+        parser.add_argument('--initial-pkgs', dest='initial_pkgs',
+                            action='append',
+                            help="")
+        parser.add_argument('--description',
+                            help="")
+        parser.add_argument('--instructions',
+                            help="")
+
+    elif action == 'list':
+        parser.add_argument("username", nargs='?',
+                            help='The username that you would like to '
+                            'list the copr of (defaults to current user)')
+
+    elif action == 'build':
+        parser.add_argument('copr', help='The copr repo to build the '
+                                         'package in')
+        parser.add_argument('pkgs', nargs='+', action='append')
+        parser.add_argument('--memory', dest='memory',
+                            help="")
+        parser.add_argument('--timeout', dest='timeout',
+                            help="")
+    return parser

-        return super(CoprCli, self).run_subcommand(argv)

def main(argv=sys.argv[1:]):
     """ Main function """
-    myapp = CoprCli()
-    return myapp.run(argv)
+    # Set up parser for global args
+    parser = setup_parser()
+    # Parse the commandline
+    arg = parser.parse_args()
+    # Get our action from these args
+    if arg.command in CMDLIST:
+        action = arg.command
+    else:
+        raise argparse.ArgumentTypeError(
+            "command must be one of: {0}".format(','.join(CMDLIST)))
+    # Parse action-specific args
+    action_parser = setup_action_parser(action, arg.argument)
+    args = action_parser.parse_args(arg.argument)
+
+    if action == "list":
+        subcommands.list(COPR_API_URL, args.username)
+
+    elif action == "create":
+        subcommands.create(COPR_API_URL, args.name, args.chroots,
+                            args.description, args.instructions,
+                            args.repos, args.initial_pkgs)
+
+    elif action == "build":
+        subcommands.build(COPR_API_URL, args.copr, args.pkgs,
+                          args.memory, args.timeout)

if __name__ == '__main__':
-    sys.exit(main(sys.argv[1:]))
-    #add_copr('test2', 'fedora-rawhide-x86_64',
-        #description='Test repos #2')
-    #list_copr()
+    try:
+        main()
+    except KeyboardInterrupt:
+        print "\nInterrupted by user."
+        sys.exit(1)
+    except argparse.ArgumentTypeError, e:
+        print "\nError: {0}".format(e)
+        sys.exit(2)
+    except Exception, e:
+        print 'Error: {0}'.format(e)
+        sys.exit(100)
diff --git a/copr_cli/subcommands.py b/copr_cli/subcommands.py
index c0013db..f2cdd5b 100644
--- a/copr_cli/subcommands.py
+++ b/copr_cli/subcommands.py
@@ -2,18 +2,10 @@

import ConfigParser
 import json
-import logging
 import requests
 import os
 import sys

-import cliff.lister
-import cliff.show
-
-from cliff.command import Command
-
-from main import copr_api_url
-

def set_user():
     """ Retrieve the user information from the config file. """
@@ -25,124 +17,83 @@ def set_user():
     return {'username': username, 'token': token}

-class List(cliff.lister.Lister):
+def list(copr_api_url, username=None):
     """ List all the copr of a user. """
-
-    log = logging.getLogger(__name__)
-
-    def get_parser(self, prog_name):
-        parser = super(type(self), self).get_parser(prog_name)
-        parser.add_argument("username", nargs='?')
-        return parser
-
-    def take_action(self, args):
-        user = set_user()
-
-        if args.username:
-            user['username'] = args.username
-        URL = '{0}/owned/'.format(copr_api_url)
-        req = requests.get(URL, params=user)
-        output = json.loads(req.text)
-        if 'repos' in output:
-            if output['repos']:
-                columns = ['name', 'description', 'repos', 'instructions']
-                values = []
-                for entry in output['repos']:
-                    values.append([entry[key] for key in columns])
-                return (columns, values)
-            else:
-                columns = ['output']
-                values = ['No copr retrieved for user: "{0}"'.format(
-                    user['username'])]
-                return (columns, [values])
+    user = set_user()
+
+    if username:
+        user['username'] = username
+
+    URL = '{0}/owned/'.format(copr_api_url)
+    req = requests.get(URL, params=user)
+    output = json.loads(req.text)
+    if 'repos' in output:
+        if output['repos']:
+            columns = ['name', 'description', 'repos', 'instructions']
+            values = []
+            for entry in output['repos']:
+                values.append([entry[key] for key in columns])
+            return (columns, values)
         else:
             columns = ['output']
-            values = ['Wrong output format returned by the server']
+            values = ['No copr retrieved for user: "{0}"'.format(
+                user['username'])]
             return (columns, [values])
+    else:
+        columns = ['output']
+        values = ['Wrong output format returned by the server']
+        return (columns, [values])

-class AddCopr(Command):
+def create(copr_api_url, name, chroots=[],description=None,
+           instructions=None, repos=None, initial_pkgs=None):
     """ Create a new copr. """
-
-    log = logging.getLogger(__name__)
-
-    def get_parser(self, prog_name):
-        parser = super(type(self), self).get_parser(prog_name)
-        parser.add_argument("name")
-        parser.add_argument("--chroot", dest="chroots", action='append',
-                            help="")
-        parser.add_argument('--repo', dest='repos', action='append',
-                            help="")
-        parser.add_argument('--initial-pkgs', dest='initial_pkgs',
-                            action='append',
-                            help="")
-        parser.add_argument('--description',
-                            help="")
-        parser.add_argument('--instructions',
-                            help="")
-        return parser
-
-    def take_action(self, args):
-        user = set_user()
-        URL = '{0}/copr/new/'.format(copr_api_url)
-
-        repos = None
-        if args.repos:
-            repos = ' '.join(args.repos)
-        initial_pkgs = None
-        if args.initial_pkgs:
-            initial_pkgs = ' '.join(args.initial_pkgs)
-        data = {'name': args.name,
-                'repos': repos,
-                'initial_pkgs': initial_pkgs,
-                'description': args.description,
-                'instructions': args.instructions
-                }
-        for chroot in args.chroots:
-            data[chroot] = 'y'
-
-        req = requests.post(URL,
-                            auth=(user['username'], user['token']),
-                            data=data)
-        output = json.loads(req.text)
-        if output['output'] == 'ok':
-            print output['message']
-        else:
-            print 'Something went wrong:\n  {0}'.format(output['error'])
-
-
-class Build(Command):
+    user = set_user()
+    URL = '{0}/copr/new/'.format(copr_api_url)
+
+    repos = None
+    if type(repos) == list():
+        repos = ' '.join(repos)
+    initial_pkgs = None
+    if type(initial_pkgs) == list():
+        initial_pkgs = ' '.join(initial_pkgs)
+    data = {'name': name,
+            'repos': repos,
+            'initial_pkgs': initial_pkgs,
+            'description': description,
+            'instructions': instructions
+            }
+    for chroot in chroots:
+        data[chroot] = 'y'
+
+    req = requests.post(URL,
+                        auth=(user['username'], user['token']),
+                        data=data)
+    output = json.loads(req.text)
+    if output['output'] == 'ok':
+        print output['message']
+    else:
+        print 'Something went wrong:\n  {0}'.format(output['error'])
+
+
+def build(copr_api_url, copr, pkgs, memory, timeout):
     """ Build a new package into a given copr. """
-
-    log = logging.getLogger(__name__)
-
-    def get_parser(self, prog_name):
-        parser = super(type(self), self).get_parser(prog_name)
-        parser.add_argument('copr')
-        parser.add_argument('pkgs', nargs='+', action='append')
-        parser.add_argument('--memory', dest='memory',
-                            help="")
-        parser.add_argument('--timeout', dest='timeout',
-                            help="")
-        return parser
-
-    def take_action(self, args):
-        user = set_user()
-        URL = '{0}/coprs/detail/{1}/{2}/new_build/'.format(
-            copr_api_url,
-            user['username'],
-            args.copr)
-
-        data = {'pkgs': ' '.join(args.pkgs[0]),
-                'memory': args.memory,
-                'timeout': args.timeout
-                }
-
-        req = requests.post(URL,
-                            auth=(user['username'], user['token']),
-                            data=data)
-        output = json.loads(req.text)
-        if req.status_code != 200:
-            print 'Something went wrong:\n  {0}'.format(output['error'])
-        else:
-            print output['message']
+    user = set_user()
+    URL = '{0}/coprs/detail/{1}/{2}/new_build/'.format(
+        copr_api_url,
+        user['username'],
+        copr)
+
+    data = {'pkgs': ' '.join(pkgs),
+            'memory': memory,
+            'timeout': timeout
+            }
+
+    req = requests.post(URL,
+                        auth=(user['username'], user['token']),
+                        data=data)
+    output = json.loads(req.text)
+    if req.status_code != 200:
+        print 'Something went wrong:\n  {0}'.format(output['error'])
+    else:
+        print output['message']
diff --git a/coprcli-setup.py b/coprcli-setup.py
index 0084494..ed7eb57 100644
--- a/coprcli-setup.py
+++ b/coprcli-setup.py
@@ -1,11 +1,6 @@
-#!/usr/bin/env python
+#!/usr/bin/python

-try:
-    from setuptools import setup
-except ImportError:
-    from ez_setup import use_setuptools
-    use_setuptools()
-    from setuptools import setup
+from setuptools import setup

import sys

@@ -13,25 +8,13 @@ f = open('README')
 long_description = f.read().strip()
 f.close()

-# Ridiculous as it may seem, we need to import multiprocessing and
-# logging here in order to get tests to pass smoothly on python 2.7.
-try:
-    import multiprocessing
-    import logging
-except ImportError:
-    pass

from copr_cli.main import __description__, __version__

requires = [
-    'cliff',
+    'requests',
 ]

-subcommands = [
-    'list = copr_cli.subcommands:List',
-    'add-copr = copr_cli.subcommands:AddCopr',
-    'build-copr = copr_cli.subcommands:Build',
-]

__name__ = 'copr-cli'
 __version__ = __version__
@@ -40,6 +23,7 @@ __author__ = "Pierre-Yves Chibon"
 __author_email__ = "[e-mail]"
 __url__ = "http://fedorahosted.org/copr/"

+
 setup(
     name=__name__,
     version=__version__,
@@ -64,7 +48,6 @@ setup(
     entry_points={
         'console_scripts': [
             'copr-cli = copr_cli.main:main'
-        ],
-        'copr_cli.subcommands': subcommands,
+        ]
     },
 )

[PATCH2/4]Removethelogfilegeneratedbycliff

[Pierre-Yves Chibon]

This log file should not have been in the git repo in the first place anyway.
---
 copr_cli/main.log | 5 -----
 1 file changed, 5 deletions(-)
 delete mode 100644 copr_cli/main.log

diff --git a/copr_cli/main.log b/copr_cli/main.log
deleted file mode 100644
index e98f62a..0000000
--- a/copr_cli/main.log
+++ /dev/null
@@ -1,5 +0,0 @@
-[2013-01-27 14:15:46,649] DEBUG    __main__ initialize_app
-[2013-01-27 14:16:02,242] DEBUG    __main__ initialize_app
-[2013-01-27 14:16:33,725] DEBUG    __main__ initialize_app
-[2013-01-27 14:16:57,768] DEBUG    __main__ initialize_app
-[2013-01-27 14:30:07,034] DEBUG    __main__ initialize_app

[PATCH3/4]Noshebangforwhatdoesnotgoin/usr/bin

[Pierre-Yves Chibon]

---
 copr_cli/main.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/copr_cli/main.py b/copr_cli/main.py
index a06e5b3..5cc7e51 100644
--- a/copr_cli/main.py
+++ b/copr_cli/main.py
@@ -1,4 +1,3 @@
-#!/usr/bin/python -tt
 #-*- coding: UTF-8 -*-

import argparse
@@ -73,6 +72,7 @@ def setup_action_parser(action, last_args=None):
                             help="")
         parser.add_argument('--timeout', dest='timeout',
                             help="")
+
     return parser

[PATCH4/4]MovetheURLtocoprintotheconfigfile

[Pierre-Yves Chibon]

This way one can use another copr server than the one provided by the Fedora Project.

Additionnaly, we rename the function set_user to get_user, semantically more appropriate.
---
 copr_cli/main.py        | 13 +++++--------
 copr_cli/subcommands.py | 31 ++++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/copr_cli/main.py b/copr_cli/main.py
index 5cc7e51..51da88e 100644
--- a/copr_cli/main.py
+++ b/copr_cli/main.py
@@ -9,9 +9,6 @@ __version__ = '0.1.0'
 __description__ = "CLI tool to run copr"

-COPR_URL = 'http://copr-fe.cloud.fedoraproject.org/'
-COPR_API_URL = '{0}/api'.format(COPR_URL)
-
 CMDLIST = ['create', 'build', 'list', 'info']

@@ -94,15 +91,15 @@ def main(argv=sys.argv[1:]):
     args = action_parser.parse_args(arg.argument)

if action == "list":
-        subcommands.list(COPR_API_URL, args.username)
+        subcommands.list(args.username)

elif action == "create":
-        subcommands.create(COPR_API_URL, args.name, args.chroots,
-                            args.description, args.instructions,
-                            args.repos, args.initial_pkgs)
+        subcommands.create(args.name, args.chroots, args.description,
+                            args.instructions, args.repos,
+                            args.initial_pkgs)

elif action == "build":
-        subcommands.build(COPR_API_URL, args.copr, args.pkgs,
+        subcommands.build(args.copr, args.pkgs,
                           args.memory, args.timeout)

diff --git a/copr_cli/subcommands.py b/copr_cli/subcommands.py
index f2cdd5b..ab494a1 100644
--- a/copr_cli/subcommands.py
+++ b/copr_cli/subcommands.py
@@ -7,7 +7,7 @@ import os
 import sys

-def set_user():
+def get_user():
     """ Retrieve the user information from the config file. """
     config = ConfigParser.ConfigParser()
     config.read(os.path.join(os.path.expanduser('~'), '.config',
@@ -17,14 +17,25 @@ def set_user():
     return {'username': username, 'token': token}

-def list(copr_api_url, username=None):
+def get_api_url():
+    """ Retrieve the user information from the config file. """
+    config = ConfigParser.ConfigParser()
+    config.read(os.path.join(os.path.expanduser('~'), '.config',
+                'copr'))
+    copr_url = config.get('copr-cli', 'copr_url',
+                          'http://copr-fe.cloud.fedoraproject.org/')
+    return '%s/api/' % copr_url
+
+
+def list(username=None):
     """ List all the copr of a user. """
-    user = set_user()
+    user = get_user()
+    copr_api_url = get_api_url()
+    URL = '{0}/owned/'.format(copr_api_url)

if username:
         user['username'] = username

-    URL = '{0}/owned/'.format(copr_api_url)
     req = requests.get(URL, params=user)
     output = json.loads(req.text)
     if 'repos' in output:
@@ -45,10 +56,11 @@ def list(copr_api_url, username=None):
         return (columns, [values])

-def create(copr_api_url, name, chroots=[],description=None,
-           instructions=None, repos=None, initial_pkgs=None):
+def create(name, chroots=[],description=None, instructions=None,
+           repos=None, initial_pkgs=None):
     """ Create a new copr. """
-    user = set_user()
+    user = get_user()
+    copr_api_url = get_api_url()
     URL = '{0}/copr/new/'.format(copr_api_url)

repos = None
@@ -76,9 +88,10 @@ def create(copr_api_url, name, chroots=[],description=None,
         print 'Something went wrong:\n  {0}'.format(output['error'])

-def build(copr_api_url, copr, pkgs, memory, timeout):
+def build(copr, pkgs, memory, timeout):
     """ Build a new package into a given copr. """
-    user = set_user()
+    user = get_user()
+    copr_api_url = get_api_url()
     URL = '{0}/coprs/detail/{1}/{2}/new_build/'.format(
         copr_api_url,
         user['username'],

[PATCH4/4]MovetheURLtocoprintotheconfigfile

[Bohuslav Kabrda]

All four patches look fine to me, please go ahead and commit (if you were waiting for my review, please don't wait next time - at least not with copr_cli related stuff, I'd only like to review patches that go into the frontend itself).

Thanks for your work on this!

[PATCH4/4]MovetheURLtocoprintotheconfigfile

[Pierre-Yves Chibon]

On Mon, Feb 18, 2013 at 01:53:53AM -0500, Bohuslav Kabrda wrote:
> All four patches look fine to me, please go ahead and commit (if you were waiting for my review, please don't wait next time - at least not with copr_cli related stuff, I'd only like to review patches that go into the frontend itself).

Noted, but I was hoping to get some feed-back on the approach I took (using
argparse, the way I handle subcommands & all).
Also, now that I think about it, I realize I need to tweak some things before
pushing all this, I do not provide the same user experience with these patches
as we have now, I need to fix this first.

Thanks for the feedback,
Pierre

[PATCH4/4]MovetheURLtocoprintotheconfigfile

[Bohuslav Kabrda]

----- Original Message -----
> On Mon, Feb 18, 2013 at 01:53:53AM -0500, Bohuslav Kabrda wrote:
> > All four patches look fine to me, please go ahead and commit (if
> > you were waiting for my review, please don't wait next time - at
> > least not with copr_cli related stuff, I'd only like to review
> > patches that go into the frontend itself).
>
> Noted, but I was hoping to get some feed-back on the approach I took
> (using
> argparse, the way I handle subcommands & all).

What about using subparsers? [1] I've used them quite successfully in one of my projects (although there is an ugly regression from python2 to python3 - in python2, subparsers can't be made optional; in python3, they are optional by default - but I think you just need to get a subparser for each action and that's it).
> Also, now that I think about it, I realize I need to tweak some
> things before
> pushing all this, I do not provide the same user experience with
> these patches
> as we have now, I need to fix this first.
>
> Thanks for the feedback,
> Pierre

[PATCH4/4]MovetheURLtocoprintotheconfigfile

[Pierre-Yves Chibon]

On Mon, Feb 18, 2013 at 02:53:22AM -0500, Bohuslav Kabrda wrote:
> ----- Original Message -----
> > On Mon, Feb 18, 2013 at 01:53:53AM -0500, Bohuslav Kabrda wrote:
> > > All four patches look fine to me, please go ahead and commit (if
> > > you were waiting for my review, please don't wait next time - at
> > > least not with copr_cli related stuff, I'd only like to review
> > > patches that go into the frontend itself).
> >
> > Noted, but I was hoping to get some feed-back on the approach I took
> > (using
> > argparse, the way I handle subcommands & all).
>
> What about using subparsers? [1] I've used them quite successfully in one of my projects (although there is an ugly regression from python2 to python3 - in python2, subparsers can't be made optional; in python3, they are optional by default - but I think you just need to get a subparser for each action and that's it).
> [1] http://docs.python.org/dev/library/argparse.html#sub-commands

Looks like exactly what I was looking for, I coded this on the train with no
internet, so I relied on what I used in pkgdb-cli but I wasn't really happy with
it. This looks much nicer, I'll give it a try.

Pierre

[PATCH4/4]MovetheURLtocoprintotheconfigfile

[Seth Vidal]

On Mon, 18 Feb 2013, Pierre-Yves Chibon wrote:
> On Mon, Feb 18, 2013 at 02:53:22AM -0500, Bohuslav Kabrda wrote:
> > ----- Original Message -----
> > > On Mon, Feb 18, 2013 at 01:53:53AM -0500, Bohuslav Kabrda wrote:
> > > > All four patches look fine to me, please go ahead and commit (if
> > > > you were waiting for my review, please don't wait next time - at
> > > > least not with copr_cli related stuff, I'd only like to review
> > > > patches that go into the frontend itself).
> > >
> > > Noted, but I was hoping to get some feed-back on the approach I took
> > > (using
> > > argparse, the way I handle subcommands & all).
> >
> > What about using subparsers? [1] I've used them quite successfully in one of my projects (although there is an ugly regression from python2 to python3 - in python2, subparsers can't be made optional; in python3, they are optional by default - but I think you just need to get a subparser for each action and that's it).
> > [1] http://docs.python.org/dev/library/argparse.html#sub-commands
>
> Looks like exactly what I was looking for, I coded this on the train with no
> internet, so I relied on what I used in pkgdb-cli but I wasn't really happy with
> it. This looks much nicer, I'll give it a try.

+1 to the subparsers. To be sure, do they function the same in rhel6 as in 
fedora 18?

-sv

[PATCH4/4]MovetheURLtocoprintotheconfigfile

[Bohuslav Kabrda]

----- Original Message -----
> On Mon, 18 Feb 2013, Pierre-Yves Chibon wrote:
> > On Mon, Feb 18, 2013 at 02:53:22AM -0500, Bohuslav Kabrda wrote:
> > > ----- Original Message -----
> > > > On Mon, Feb 18, 2013 at 01:53:53AM -0500, Bohuslav Kabrda wrote:
> > > > > All four patches look fine to me, please go ahead and commit (if
> > > > > you were waiting for my review, please don't wait next time - at
> > > > > least not with copr_cli related stuff, I'd only like to review
> > > > > patches that go into the frontend itself).
> > > >
> > > > Noted, but I was hoping to get some feed-back on the approach I
> > > > took
> > > > (using
> > > > argparse, the way I handle subcommands & all).
> > >
> > > What about using subparsers? [1] I've used them quite successfully
> > > in one of my projects (although there is an ugly regression from
> > > python2 to python3 - in python2, subparsers can't be made
> > > optional; in python3, they are optional by default - but I think
> > > you just need to get a subparser for each action and that's it).
> > > [1] http://docs.python.org/dev/library/argparse.html#sub-commands
> >
> > Looks like exactly what I was looking for, I coded this on the
> > train with no
> > internet, so I relied on what I used in pkgdb-cli but I wasn't
> > really happy with
> > it. This looks much nicer, I'll give it a try.
>
> +1 to the subparsers. To be sure, do they function the same in rhel6
> as in
> fedora 18?
>
> -sv

AFAIK the only regression is between Python2 and Python3 builtin argparse. Python2.7 builtin argparse should be pretty much the same as the standalone argparse used with Python2.6 (so yes, they function the same).