openstack/python-openstackclient
Code Issues Proposed changes

Fix NoneType error for volume snapshot create command

Browse Source 
In volume snapshot command, <volume> is the same
as <snapshot-name> when --volume is not specified,
but <volume> cannot be None, so when <snapshot-name>
is not specified (<snapshot-name> is None), a NoneType
error appears.
So make <snapshot-name> no longer optional, it should
be always present.

Change-Id: I3d9f10753a8ef601e70816421c160598e2cc811f
Closes-bug: #1659894
This commit is contained in:
Huanxuan Ao 2017-03-08 10:01:16 +08:00 committed by Dean Troyer
parent bf1f47c1be
commit 1c49a1f01d
7 changed files with 42 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
doc/source/backwards-incompatible.rst
View File

@ -27,6 +27,21 @@ Backwards Incompatible Changes
.. * Remove in: <5.0>
.. * Commit: <tbd>
Release 3.10
------------
1. The positional argument ``<snapshot-name>`` of the ``volume snapshot create``
command is no longer optional.
Previously when the ``--volume`` option was
present ``<snapshot-name>`` defaulted to the ``--volume`` value. When the
``--volume`` option is not present now it defaults to the value of
``<snapshot-name>``.
* As of: 3.10
* Bug: 1659894
* Commit: https://review.openstack.org/440497
Release 3.0
-----------

2
doc/source/command-objects/volume-snapshot.rst
View File

@ -49,7 +49,7 @@ Create new volume snapshot
.. _volume_snapshot_create-snapshot-name:
.. describe:: <snapshot-name>
Name of the new snapshot (default to None)
Name of the new snapshot
volume snapshot delete
----------------------

22
openstackclient/tests/unit/volume/v1/test_snapshot.py
View File

@ -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 = [

29
openstackclient/tests/unit/volume/v2/test_snapshot.py
View File

@ -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,
)

3
openstackclient/volume/v1/volume_snapshot.py
View File

@ -38,8 +38,7 @@ class CreateVolumeSnapshot(command.ShowOne):
parser.add_argument(
'snapshot_name',
metavar='<snapshot-name>',
nargs="?",
help=_('Name of the snapshot (default to None)'),
help=_('Name of the new snapshot'),
)
parser.add_argument(
'--volume',

3
openstackclient/volume/v2/volume_snapshot.py
View File

@ -38,8 +38,7 @@ class CreateVolumeSnapshot(command.ShowOne):
parser.add_argument(
"snapshot_name",
metavar="<snapshot-name>",
nargs="?",
help=_("Name of the new snapshot (default to None)")
help=_("Name of the new snapshot"),
)
parser.add_argument(
"--volume",

6
releasenotes/notes/bug-1659894-4518b10615498ba9.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
Now the positional parameter ``<snapshot-name>`` of ``volume snapshot create``
command is no longer optional, it should be always present.
[Bug `1659894 <https://bugs.launchpad.net/bugs/1659894>`_]