From 59dd49978c04bb19dd7f56e1e97a2bb75a0da759 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 5 Apr 2017 18:17:21 +0100 Subject: [PATCH] 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 | 20 ++++++++----- nova/cmd/manage.py | 29 +++++++++++++++---- ...a-manage-db-commands-b958b0a41a4004a6.yaml | 20 +++++++++++++ 3 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/reworked-nova-manage-db-commands-b958b0a41a4004a6.yaml diff --git a/nova/cmd/common.py b/nova/cmd/common.py index 260685ef77..494483f97b 100644 --- a/nova/cmd/common.py +++ b/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 - # the leading hyphens if no dest is supplied - kwargs.setdefault('dest', args[0][2:]) - if kwargs['dest'].startswith('action_kwarg_'): - action_kwargs.append(kwargs['dest'][len('action_kwarg_'):]) + # we must handle positional parameters (ARG) separately from + # positional parameters (--opt). Detect this by checking for + # the presence of leading '--' + if args[0] != args[0].lstrip('-'): + 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']) - kwargs['dest'] = 'action_kwarg_' + kwargs['dest'] + action_kwargs.append(args[0]) + args = ['action_kwarg_' + arg for arg in args] parser.add_argument(*args, **kwargs) diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 83ecfa85c1..4fae174a60 100644 --- a/nova/cmd/manage.py +++ b/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='', help='Database version') + @args('--version', metavar='', 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='', help='Database version') - def sync(self, version=None): + @args('--version', metavar='', help=argparse.SUPPRESS) + @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.""" diff --git a/releasenotes/notes/reworked-nova-manage-db-commands-b958b0a41a4004a6.yaml b/releasenotes/notes/reworked-nova-manage-db-commands-b958b0a41a4004a6.yaml new file mode 100644 index 0000000000..1112b93e86 --- /dev/null +++ b/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.