From 1c49a1f01da73b8eed701809de88b408e738dfed Mon Sep 17 00:00:00 2001 From: Huanxuan Ao Date: Wed, 8 Mar 2017 10:01:16 +0800 Subject: [PATCH] Fix NoneType error for volume snapshot create command In volume snapshot command, is the same as when --volume is not specified, but cannot be None, so when is not specified ( is None), a NoneType error appears. So make no longer optional, it should be always present. Change-Id: I3d9f10753a8ef601e70816421c160598e2cc811f Closes-bug: #1659894 --- doc/source/backwards-incompatible.rst | 15 ++++++++++ .../command-objects/volume-snapshot.rst | 2 +- .../tests/unit/volume/v1/test_snapshot.py | 22 +++++--------- .../tests/unit/volume/v2/test_snapshot.py | 29 +++++++------------ openstackclient/volume/v1/volume_snapshot.py | 3 +- openstackclient/volume/v2/volume_snapshot.py | 3 +- .../notes/bug-1659894-4518b10615498ba9.yaml | 6 ++++ 7 files changed, 42 insertions(+), 38 deletions(-) create mode 100644 releasenotes/notes/bug-1659894-4518b10615498ba9.yaml diff --git a/doc/source/backwards-incompatible.rst b/doc/source/backwards-incompatible.rst index 22c300de0..0202e8a5a 100644 --- a/doc/source/backwards-incompatible.rst +++ b/doc/source/backwards-incompatible.rst @@ -27,6 +27,21 @@ Backwards Incompatible Changes .. * Remove in: <5.0> .. * Commit: +Release 3.10 +------------ + +1. The positional argument ```` of the ``volume snapshot create`` + command is no longer optional. + + Previously when the ``--volume`` option was + present ```` defaulted to the ``--volume`` value. When the + ``--volume`` option is not present now it defaults to the value of + ````. + + * As of: 3.10 + * Bug: 1659894 + * Commit: https://review.openstack.org/440497 + Release 3.0 ----------- diff --git a/doc/source/command-objects/volume-snapshot.rst b/doc/source/command-objects/volume-snapshot.rst index 67db62f2b..30cc77cc5 100644 --- a/doc/source/command-objects/volume-snapshot.rst +++ b/doc/source/command-objects/volume-snapshot.rst @@ -49,7 +49,7 @@ Create new volume snapshot .. _volume_snapshot_create-snapshot-name: .. describe:: - Name of the new snapshot (default to None) + Name of the new snapshot volume snapshot delete ---------------------- diff --git a/openstackclient/tests/unit/volume/v1/test_snapshot.py b/openstackclient/tests/unit/volume/v1/test_snapshot.py index 87a62b0a9..70b55ce23 100644 --- a/openstackclient/tests/unit/volume/v1/test_snapshot.py +++ b/openstackclient/tests/unit/volume/v1/test_snapshot.py @@ -18,6 +18,7 @@ from mock import call from osc_lib import exceptions from osc_lib import utils +from openstackclient.tests.unit import utils as tests_utils from openstackclient.tests.unit.volume.v1 import fakes as volume_fakes from openstackclient.volume.v1 import volume_snapshot @@ -98,26 +99,17 @@ class TestSnapshotCreate(TestSnapshot): def test_snapshot_create_without_name(self): arglist = [ "--volume", self.new_snapshot.volume_id, - "--description", self.new_snapshot.display_description, - "--force" ] verifylist = [ ("volume", self.new_snapshot.volume_id), - ("description", self.new_snapshot.display_description), - ("force", True) ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - columns, data = self.cmd.take_action(parsed_args) - - self.snapshots_mock.create.assert_called_with( - self.new_snapshot.volume_id, - True, - None, - self.new_snapshot.display_description, + self.assertRaises( + tests_utils.ParserException, + self.check_parser, + self.cmd, + arglist, + verifylist, ) - self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) def test_snapshot_create_without_volume(self): arglist = [ diff --git a/openstackclient/tests/unit/volume/v2/test_snapshot.py b/openstackclient/tests/unit/volume/v2/test_snapshot.py index 1ad97e858..16d0602b7 100644 --- a/openstackclient/tests/unit/volume/v2/test_snapshot.py +++ b/openstackclient/tests/unit/volume/v2/test_snapshot.py @@ -20,6 +20,7 @@ from osc_lib import exceptions from osc_lib import utils from openstackclient.tests.unit.identity.v3 import fakes as project_fakes +from openstackclient.tests.unit import utils as tests_utils from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes from openstackclient.volume.v2 import volume_snapshot @@ -107,27 +108,17 @@ class TestSnapshotCreate(TestSnapshot): def test_snapshot_create_without_name(self): arglist = [ "--volume", self.new_snapshot.volume_id, - "--description", self.new_snapshot.description, - "--force" ] verifylist = [ ("volume", self.new_snapshot.volume_id), - ("description", self.new_snapshot.description), - ("force", True) ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - columns, data = self.cmd.take_action(parsed_args) - - self.snapshots_mock.create.assert_called_with( - self.new_snapshot.volume_id, - force=True, - name=None, - description=self.new_snapshot.description, - metadata=None, + self.assertRaises( + tests_utils.ParserException, + self.check_parser, + self.cmd, + arglist, + verifylist, ) - self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) def test_snapshot_create_without_volume(self): arglist = [ @@ -156,17 +147,19 @@ class TestSnapshotCreate(TestSnapshot): self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) - def test_snapshot_create_without_remote_source(self): + def test_snapshot_create_with_remote_source(self): arglist = [ '--remote-source', 'source-name=test_source_name', '--remote-source', 'source-id=test_source_id', '--volume', self.new_snapshot.volume_id, + self.new_snapshot.name, ] ref_dict = {'source-name': 'test_source_name', 'source-id': 'test_source_id'} verifylist = [ ('remote_source', ref_dict), ('volume', self.new_snapshot.volume_id), + ("snapshot_name", self.new_snapshot.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -175,7 +168,7 @@ class TestSnapshotCreate(TestSnapshot): self.snapshots_mock.manage.assert_called_with( volume_id=self.new_snapshot.volume_id, ref=ref_dict, - name=None, + name=self.new_snapshot.name, description=None, metadata=None, ) diff --git a/openstackclient/volume/v1/volume_snapshot.py b/openstackclient/volume/v1/volume_snapshot.py index f22c338b8..3e83da5a7 100644 --- a/openstackclient/volume/v1/volume_snapshot.py +++ b/openstackclient/volume/v1/volume_snapshot.py @@ -38,8 +38,7 @@ class CreateVolumeSnapshot(command.ShowOne): parser.add_argument( 'snapshot_name', metavar='', - nargs="?", - help=_('Name of the snapshot (default to None)'), + help=_('Name of the new snapshot'), ) parser.add_argument( '--volume', diff --git a/openstackclient/volume/v2/volume_snapshot.py b/openstackclient/volume/v2/volume_snapshot.py index 804c82911..fe9694104 100644 --- a/openstackclient/volume/v2/volume_snapshot.py +++ b/openstackclient/volume/v2/volume_snapshot.py @@ -38,8 +38,7 @@ class CreateVolumeSnapshot(command.ShowOne): parser.add_argument( "snapshot_name", metavar="", - nargs="?", - help=_("Name of the new snapshot (default to None)") + help=_("Name of the new snapshot"), ) parser.add_argument( "--volume", diff --git a/releasenotes/notes/bug-1659894-4518b10615498ba9.yaml b/releasenotes/notes/bug-1659894-4518b10615498ba9.yaml new file mode 100644 index 000000000..e1afbb45a --- /dev/null +++ b/releasenotes/notes/bug-1659894-4518b10615498ba9.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Now the positional parameter ```` of ``volume snapshot create`` + command is no longer optional, it should be always present. + [Bug `1659894 `_]