From 4a07e63e7ea4b99b10e4a3fd9ed06c0cf6ee905f Mon Sep 17 00:00:00 2001 From: lin-hua-cheng Date: Fri, 19 Dec 2014 18:40:40 -0800 Subject: [PATCH] type should be required for v2.0 service create Updated the service name to be optional, mostly matching the cli arguments with v3 service create. Implemented the following changes on service create: - if only a single positional is present, it's a . This is not currently legal so it is considered a new case. - if --type option is present the positional is handled as ; display deprecation message - if --name option is present the positional is handled as . Making --type optional is new, but back-compatible - Made --name and --type mutually exclusive. - only '--name ' shall appear in the help output Change-Id: I8fd4adba3d8cd00d5a8cacc2c494d99d492c45a3 Closes-Bug: #1404073 --- openstackclient/identity/v2_0/service.py | 41 ++++++++-- .../tests/identity/v2_0/test_service.py | 75 ++++++++++++++++++- 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/openstackclient/identity/v2_0/service.py b/openstackclient/identity/v2_0/service.py index e8848ddee2..0b98a90360 100644 --- a/openstackclient/identity/v2_0/service.py +++ b/openstackclient/identity/v2_0/service.py @@ -15,6 +15,7 @@ """Service action implementations""" +import argparse import logging import six @@ -36,15 +37,20 @@ class CreateService(show.ShowOne): def get_parser(self, prog_name): parser = super(CreateService, self).get_parser(prog_name) parser.add_argument( - 'name', - metavar='', - help=_('New service name'), + 'type_or_name', + metavar='', + help=_('New service type (compute, image, identity, volume, etc)'), ) - parser.add_argument( + type_or_name_group = parser.add_mutually_exclusive_group() + type_or_name_group.add_argument( '--type', metavar='', - required=True, - help=_('New service type (compute, image, identity, volume, etc)'), + help=argparse.SUPPRESS, + ) + type_or_name_group.add_argument( + '--name', + metavar='', + help=_('New service name'), ) parser.add_argument( '--description', @@ -57,9 +63,28 @@ class CreateService(show.ShowOne): self.log.debug('take_action(%s)', parsed_args) identity_client = self.app.client_manager.identity + type_or_name = parsed_args.type_or_name + name = parsed_args.name + type = parsed_args.type + + # If only a single positional is present, it's a . + # This is not currently legal so it is considered a new case. + if not type and not name: + type = type_or_name + # If --type option is present then positional is handled as ; + # display deprecation message. + elif type: + name = type_or_name + self.log.warning(_('The argument --type is deprecated, use service' + ' create --name type instead.')) + # If --name option is present the positional is handled as . + # Making --type optional is new, but back-compatible + elif name: + type = type_or_name + service = identity_client.services.create( - parsed_args.name, - parsed_args.type, + name, + type, parsed_args.description) info = {} diff --git a/openstackclient/tests/identity/v2_0/test_service.py b/openstackclient/tests/identity/v2_0/test_service.py index 6c93574bf5..a0adea4e9e 100644 --- a/openstackclient/tests/identity/v2_0/test_service.py +++ b/openstackclient/tests/identity/v2_0/test_service.py @@ -44,14 +44,80 @@ class TestServiceCreate(TestService): # Get the command object to test self.cmd = service.CreateService(self.app, None) - def test_service_create_name_type(self): + def test_service_create_with_type_positional(self): + arglist = [ + identity_fakes.service_type, + ] + verifylist = [ + ('type_or_name', identity_fakes.service_type), + ('type', None), + ('description', None), + ('name', None), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # DisplayCommandBase.take_action() returns two tuples + columns, data = self.cmd.take_action(parsed_args) + + # ServiceManager.create(name, service_type, description) + self.services_mock.create.assert_called_with( + None, + identity_fakes.service_type, + None, + ) + + collist = ('description', 'id', 'name', 'type') + self.assertEqual(columns, collist) + datalist = ( + identity_fakes.service_description, + identity_fakes.service_id, + identity_fakes.service_name, + identity_fakes.service_type, + ) + self.assertEqual(data, datalist) + + def test_service_create_with_type_option(self): arglist = [ '--type', identity_fakes.service_type, identity_fakes.service_name, ] verifylist = [ + ('type_or_name', identity_fakes.service_name), ('type', identity_fakes.service_type), ('description', None), + ('name', None), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # DisplayCommandBase.take_action() returns two tuples + columns, data = self.cmd.take_action(parsed_args) + + # ServiceManager.create(name, service_type, description) + self.services_mock.create.assert_called_with( + identity_fakes.service_name, + identity_fakes.service_type, + None, + ) + + collist = ('description', 'id', 'name', 'type') + self.assertEqual(columns, collist) + datalist = ( + identity_fakes.service_description, + identity_fakes.service_id, + identity_fakes.service_name, + identity_fakes.service_type, + ) + self.assertEqual(data, datalist) + + def test_service_create_with_name_option(self): + arglist = [ + '--name', identity_fakes.service_name, + identity_fakes.service_type, + ] + verifylist = [ + ('type_or_name', identity_fakes.service_type), + ('type', None), + ('description', None), ('name', identity_fakes.service_name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -78,12 +144,13 @@ class TestServiceCreate(TestService): def test_service_create_description(self): arglist = [ - '--type', identity_fakes.service_type, + '--name', identity_fakes.service_name, '--description', identity_fakes.service_description, - identity_fakes.service_name, + identity_fakes.service_type, ] verifylist = [ - ('type', identity_fakes.service_type), + ('type_or_name', identity_fakes.service_type), + ('type', None), ('description', identity_fakes.service_description), ('name', identity_fakes.service_name), ]