nova-manage: Deprecate '--version' parameters

Both the 'api_db' and 'db' categories / commands define a 'sync'
(sub)command which takes an optional '--version' parameter. We want to
start using 'cliff' in Queens, but that framework does not support
command-level parameters with the exception of '--help' [1]. If you
think of something like the 'ls' command, you can run it without any
arguments and assume a default of the current directory, or you can
specify the argument. In the same way, these commands should really be
using optional parameters instead of positional arguments. We do this
and add aliases for the older parameters to ease with the transition.
These aliases are deprecated, raise warnings and will be removed in the
move to cliff.

[1] https://bugs.launchpad.net/python-cliff/+bug/1619708

Change-Id: I3fd9fe0317bcd1a59c366e60154b095e8df92327
Partially-Implements: bp move-nova-cmds-to-cliff

nova/cmd/common.py

@@ -102,14 +102,20 @@ def add_command_parsers(subparsers, categories):
 
             action_kwargs = []
             for args, kwargs in getattr(action_fn, 'args', []):
-                # FIXME(markmc): hack to assume dest is the arg name without
+                # we must handle positional parameters (ARG) separately from
-                # the leading hyphens if no dest is supplied
+                # positional parameters (--opt). Detect this by checking for
-                kwargs.setdefault('dest', args[0][2:])
+                # the presence of leading '--'
-                if kwargs['dest'].startswith('action_kwarg_'):
+                if args[0] != args[0].lstrip('-'):
-                    action_kwargs.append(kwargs['dest'][len('action_kwarg_'):])
+                    kwargs.setdefault('dest', args[0].lstrip('-'))
+                    if kwargs['dest'].startswith('action_kwarg_'):
+                        action_kwargs.append(
+                            kwargs['dest'][len('action_kwarg_'):])
+                    else:
+                        action_kwargs.append(kwargs['dest'])
+                        kwargs['dest'] = 'action_kwarg_' + kwargs['dest']
                 else:
-                    action_kwargs.append(kwargs['dest'])
+                    action_kwargs.append(args[0])
-                    kwargs['dest'] = 'action_kwarg_' + kwargs['dest']
+                    args = ['action_kwarg_' + arg for arg in args]
 
                 parser.add_argument(*args, **kwargs)
 

nova/cmd/manage.py

@@ -54,6 +54,7 @@
 
 from __future__ import print_function
 
+import argparse
 import functools
 import os
 import re
@@ -670,12 +671,20 @@ class DbCommands(object):
     def __init__(self):
         pass
 
-    @args('--version', metavar='<version>', help='Database version')
+    @args('--version', metavar='<version>', help=argparse.SUPPRESS)
     @args('--local_cell', action='store_true',
           help='Only sync db in the local cell: do not attempt to fan-out'
                'to all cells')
-    def sync(self, version=None, local_cell=False):
+    @args('version2', metavar='VERSION', nargs='?', help='Database version')
+    def sync(self, version=None, local_cell=False, version2=None):
         """Sync the database up to the most recent version."""
+        if version and not version2:
+            print(_("DEPRECATED: The '--version' parameter was deprecated in "
+                    "the Pike cycle and will not be supported in future "
+                    "versions of nova. Use the 'VERSION' positional argument "
+                    "instead"))
+            version2 = version
+
         if not local_cell:
             ctxt = context.RequestContext()
             # NOTE(mdoff): Multiple cells not yet implemented. Currently
@@ -684,7 +693,7 @@ class DbCommands(object):
                 cell_mapping = objects.CellMapping.get_by_uuid(ctxt,
                                             objects.CellMapping.CELL0_UUID)
                 with context.target_cell(ctxt, cell_mapping) as cctxt:
-                    migration.db_sync(version, context=cctxt)
+                    migration.db_sync(version2, context=cctxt)
             except exception.CellMappingNotFound:
                 print(_('WARNING: cell0 mapping not found - not'
                         ' syncing cell0.'))
@@ -853,10 +862,18 @@ class ApiDbCommands(object):
     def __init__(self):
         pass
 
-    @args('--version', metavar='<version>', help='Database version')
+    @args('--version', metavar='<version>', help=argparse.SUPPRESS)
-    def sync(self, version=None):
+    @args('version2', metavar='VERSION', nargs='?', help='Database version')
+    def sync(self, version=None, version2=None):
         """Sync the database up to the most recent version."""
-        return migration.db_sync(version, database='api')
+        if version and not version2:
+            print(_("DEPRECATED: The '--version' parameter was deprecated in "
+                    "the Pike cycle and will not be supported in future "
+                    "versions of nova. Use the 'VERSION' positional argument "
+                    "instead"))
+            version2 = version
+
+        return migration.db_sync(version2, database='api')
 
     def version(self):
         """Print the current database version."""

releasenotes/notes/reworked-nova-manage-db-commands-b958b0a41a4004a6.yaml

@@ -0,0 +1,20 @@
+---
+upgrade:
+  - |
+    The ``nova-manage api_db sync`` and ``nova-manage db sync`` commands
+    previously took an optional ``--version`` parameter to determine which
+    version to sync to. For example::
+
+        $ nova-manage api_db sync --version some-version
+
+    This is now an optional positional argument. For example::
+
+        $ nova-manage api_db sync some-version
+
+    Aliases are provided but these are marked as deprecated and will be removed
+    in the next release of nova.
+deprecations:
+  - |
+    The ``--version`` parameters of the ``nova-manage api_db sync`` and
+    ``nova-manage db sync`` commands has been deprecated in favor of
+    positional arguments.