openstack/python-openstackclient
Code Issues Proposed changes

Close files on image create

Browse Source 
The file opened for --file was never closed.  Close it if it is a
file object.

Change-Id: I7bd120a2413de42339771d01e8fd1894d38c3011
This commit is contained in:
Dean Troyer 2014-10-13 22:40:11 -05:00
parent 36212c43d8
commit ca783f4659
2 changed files with 40 additions and 26 deletions
openstackclient
image/v1
image.py
tests/image/v1
test_image.py

49
openstackclient/image/v1/image.py

@ -15,6 +15,7 @@
"""Image V1 Action Implementations""" """Image V1 Action Implementations"""
import io
import logging import logging
import os import os
import six import six
@ -214,10 +215,9 @@ class CreateImage(show.ShowOne):
elif parsed_args.file: elif parsed_args.file:
# Send an open file handle to glanceclient so it will # Send an open file handle to glanceclient so it will
# do a chunked transfer # do a chunked transfer
kwargs["data"] = open(parsed_args.file, "rb") kwargs["data"] = io.open(parsed_args.file, "rb")
else: else:
# Read file from stdin # Read file from stdin
kwargs["data"] = None
if sys.stdin.isatty() is not True: if sys.stdin.isatty() is not True:
if msvcrt: if msvcrt:
msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY) msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY)
@ -225,29 +225,36 @@ class CreateImage(show.ShowOne):
# do a chunked transfer # do a chunked transfer
kwargs["data"] = sys.stdin kwargs["data"] = sys.stdin
# Wrap the call to catch exceptions in order to close files
try: try:
image = utils.find_resource( try:
image_client.images, image = utils.find_resource(
parsed_args.name, image_client.images,
) parsed_args.name,
)
# Preserve previous properties if any are being set now # Preserve previous properties if any are being set now
if image.properties: if image.properties:
if parsed_args.properties: if parsed_args.properties:
image.properties.update(kwargs['properties']) image.properties.update(kwargs['properties'])
kwargs['properties'] = image.properties kwargs['properties'] = image.properties
except exceptions.CommandError: except exceptions.CommandError:
if not parsed_args.volume: if not parsed_args.volume:
# This is normal for a create or reserve (create w/o an image) # This is normal for a create or reserve (create w/o
# But skip for create from volume # an image), but skip for create from volume
image = image_client.images.create(**kwargs) image = image_client.images.create(**kwargs)
else: else:
# Update an existing reservation # Update an existing reservation
# If an image is specified via --file, --location or # If an image is specified via --file, --location or
# --copy-from let the API handle it # --copy-from let the API handle it
image = image_client.images.update(image.id, **kwargs) image = image_client.images.update(image.id, **kwargs)
finally:
# Clean up open files - make sure data isn't a string
if ('data' in kwargs and hasattr(kwargs['data'], 'close') and
kwargs['data'] != sys.stdin):
kwargs['data'].close()
info = {} info = {}
info.update(image._info) info.update(image._info)

17
openstackclient/tests/image/v1/test_image.py

@ -139,14 +139,17 @@ class TestImageCreate(TestImage):
self.assertEqual(image_fakes.IMAGE_columns, columns) self.assertEqual(image_fakes.IMAGE_columns, columns)
self.assertEqual(image_fakes.IMAGE_data, data) self.assertEqual(image_fakes.IMAGE_data, data)
@mock.patch('six.moves.builtins.open') @mock.patch('openstackclient.image.v1.image.io.open', name='Open')
def test_image_create_file(self, open_mock): def test_image_create_file(self, mock_open):
mock_file = mock.MagicMock(name='File')
mock_open.return_value = mock_file
mock_open.read.return_value = image_fakes.image_data
mock_exception = { mock_exception = {
'find.side_effect': exceptions.CommandError('x'), 'find.side_effect': exceptions.CommandError('x'),
'get.side_effect': exceptions.CommandError('x'), 'get.side_effect': exceptions.CommandError('x'),
} }
self.images_mock.configure_mock(**mock_exception) self.images_mock.configure_mock(**mock_exception)
open_mock.return_value = image_fakes.image_data
arglist = [ arglist = [
'--file', 'filer', '--file', 'filer',
'--unprotected', '--unprotected',
@ -169,7 +172,11 @@ class TestImageCreate(TestImage):
# DisplayCommandBase.take_action() returns two tuples # DisplayCommandBase.take_action() returns two tuples
columns, data = self.cmd.take_action(parsed_args) columns, data = self.cmd.take_action(parsed_args)
open_mock.assert_called_with('filer', 'rb') # Ensure input file is opened
mock_open.assert_called_with('filer', 'rb')
# Ensure the input file is closed
mock_file.close.assert_called_with()
# ImageManager.get(name) # ImageManager.get(name)
self.images_mock.get.assert_called_with(image_fakes.image_name) self.images_mock.get.assert_called_with(image_fakes.image_name)
@ -185,7 +192,7 @@ class TestImageCreate(TestImage):
'Alpha': '1', 'Alpha': '1',
'Beta': '2', 'Beta': '2',
}, },
data=image_fakes.image_data, data=mock_file,
) )
# Verify update() was not called, if it was show the args # Verify update() was not called, if it was show the args