Use the base Resource's JSON patch support in Image
Note that as a side effect, the image service resources no longer allow setting unsupported attributes. Their support was a bit awkward since they were essentially write-only. On the other hand, this change can be breaking for consumers that rely on it. Change-Id: I04dc37e0570b84c80d7c19fd090ba7043e70bd74
This commit is contained in:
		| @@ -157,8 +157,7 @@ class Proxy(proxy.Proxy): | |||||||
|         :returns: The updated image |         :returns: The updated image | ||||||
|         :rtype: :class:`~openstack.image.v2.image.Image` |         :rtype: :class:`~openstack.image.v2.image.Image` | ||||||
|         """ |         """ | ||||||
|         res = self._get_resource(_image.Image, image) |         return self._update(_image.Image, image, **attrs) | ||||||
|         return res.commit(self, **attrs) |  | ||||||
|  |  | ||||||
|     def deactivate_image(self, image): |     def deactivate_image(self, image): | ||||||
|         """Deactivate an image |         """Deactivate an image | ||||||
|   | |||||||
| @@ -12,9 +12,6 @@ | |||||||
|  |  | ||||||
| import hashlib | import hashlib | ||||||
|  |  | ||||||
| import copy |  | ||||||
| import jsonpatch |  | ||||||
|  |  | ||||||
| from openstack import _log | from openstack import _log | ||||||
| from openstack import exceptions | from openstack import exceptions | ||||||
| from openstack.image import image_service | from openstack.image import image_service | ||||||
| @@ -36,6 +33,7 @@ class Image(resource.Resource): | |||||||
|     allow_delete = True |     allow_delete = True | ||||||
|     allow_list = True |     allow_list = True | ||||||
|     commit_method = 'PATCH' |     commit_method = 'PATCH' | ||||||
|  |     commit_jsonpatch = True | ||||||
|  |  | ||||||
|     _query_mapping = resource.QueryParameters( |     _query_mapping = resource.QueryParameters( | ||||||
|         "name", "visibility", |         "name", "visibility", | ||||||
| @@ -114,12 +112,6 @@ class Image(resource.Resource): | |||||||
|     #: when you set the show_image_direct_url option to true in the |     #: when you set the show_image_direct_url option to true in the | ||||||
|     #: Image service's configuration file. |     #: Image service's configuration file. | ||||||
|     direct_url = resource.Body('direct_url') |     direct_url = resource.Body('direct_url') | ||||||
|     #: An image property. |  | ||||||
|     path = resource.Body('path') |  | ||||||
|     #: Value of image property used in add or replace operations expressed |  | ||||||
|     #: in JSON notation. For example, you must enclose strings in quotation |  | ||||||
|     #: marks, and you do not enclose numeric values in quotation marks. |  | ||||||
|     value = resource.Body('value') |  | ||||||
|     #: The URL to access the image file kept in external store. |     #: The URL to access the image file kept in external store. | ||||||
|     url = resource.Body('url') |     url = resource.Body('url') | ||||||
|     #: The location metadata. |     #: The location metadata. | ||||||
| @@ -294,21 +286,16 @@ class Image(resource.Resource): | |||||||
|  |  | ||||||
|         return resp.content |         return resp.content | ||||||
|  |  | ||||||
|     def commit(self, session, **attrs): |     def _prepare_request(self, requires_id=None, prepend_key=False, | ||||||
|         url = utils.urljoin(self.base_path, self.id) |                          patch=False): | ||||||
|         headers = { |         request = super(Image, self)._prepare_request(requires_id=requires_id, | ||||||
|             'Content-Type': 'application/openstack-images-v2.1-json-patch', |                                                       prepend_key=prepend_key, | ||||||
|             'Accept': '' |                                                       patch=patch) | ||||||
|         } |         if patch: | ||||||
|         original = self.to_dict() |             headers = { | ||||||
|  |                 'Content-Type': 'application/openstack-images-v2.1-json-patch', | ||||||
|  |                 'Accept': '' | ||||||
|  |             } | ||||||
|  |             request.headers.update(headers) | ||||||
|  |  | ||||||
|         # Update values from **attrs so they can be passed to jsonpatch |         return request | ||||||
|         new = copy.deepcopy(self.to_dict()) |  | ||||||
|         new.update(**attrs) |  | ||||||
|  |  | ||||||
|         patch_string = jsonpatch.make_patch(original, new).to_string() |  | ||||||
|         resp = session.patch(url, |  | ||||||
|                              data=patch_string, |  | ||||||
|                              headers=headers) |  | ||||||
|         self._translate_response(resp, has_body=True) |  | ||||||
|         return self |  | ||||||
|   | |||||||
| @@ -10,7 +10,6 @@ | |||||||
| # License for the specific language governing permissions and limitations | # License for the specific language governing permissions and limitations | ||||||
| # under the License. | # under the License. | ||||||
|  |  | ||||||
| import json |  | ||||||
| import operator | import operator | ||||||
|  |  | ||||||
| from keystoneauth1 import adapter | from keystoneauth1 import adapter | ||||||
| @@ -45,8 +44,6 @@ EXAMPLE = { | |||||||
|     'file': '15', |     'file': '15', | ||||||
|     'locations': ['15', '16'], |     'locations': ['15', '16'], | ||||||
|     'direct_url': '17', |     'direct_url': '17', | ||||||
|     'path': '18', |  | ||||||
|     'value': '19', |  | ||||||
|     'url': '20', |     'url': '20', | ||||||
|     'metadata': {'21': '22'}, |     'metadata': {'21': '22'}, | ||||||
|     'architecture': '23', |     'architecture': '23', | ||||||
| @@ -142,8 +139,6 @@ class TestImage(base.TestCase): | |||||||
|         self.assertEqual(EXAMPLE['file'], sot.file) |         self.assertEqual(EXAMPLE['file'], sot.file) | ||||||
|         self.assertEqual(EXAMPLE['locations'], sot.locations) |         self.assertEqual(EXAMPLE['locations'], sot.locations) | ||||||
|         self.assertEqual(EXAMPLE['direct_url'], sot.direct_url) |         self.assertEqual(EXAMPLE['direct_url'], sot.direct_url) | ||||||
|         self.assertEqual(EXAMPLE['path'], sot.path) |  | ||||||
|         self.assertEqual(EXAMPLE['value'], sot.value) |  | ||||||
|         self.assertEqual(EXAMPLE['url'], sot.url) |         self.assertEqual(EXAMPLE['url'], sot.url) | ||||||
|         self.assertEqual(EXAMPLE['metadata'], sot.metadata) |         self.assertEqual(EXAMPLE['metadata'], sot.metadata) | ||||||
|         self.assertEqual(EXAMPLE['architecture'], sot.architecture) |         self.assertEqual(EXAMPLE['architecture'], sot.architecture) | ||||||
| @@ -312,7 +307,9 @@ class TestImage(base.TestCase): | |||||||
|         self.assertEqual(rv, resp) |         self.assertEqual(rv, resp) | ||||||
|  |  | ||||||
|     def test_image_update(self): |     def test_image_update(self): | ||||||
|         sot = image.Image(**EXAMPLE) |         values = EXAMPLE.copy() | ||||||
|  |         del values['instance_uuid'] | ||||||
|  |         sot = image.Image(**values) | ||||||
|         # Let the translate pass through, that portion is tested elsewhere |         # Let the translate pass through, that portion is tested elsewhere | ||||||
|         sot._translate_response = mock.Mock() |         sot._translate_response = mock.Mock() | ||||||
|  |  | ||||||
| @@ -326,21 +323,18 @@ class TestImage(base.TestCase): | |||||||
|         resp.status_code = 200 |         resp.status_code = 200 | ||||||
|         self.sess.patch.return_value = resp |         self.sess.patch.return_value = resp | ||||||
|  |  | ||||||
|         value = ('[{"value": "fake_name", "op": "replace", "path": "/name"}, ' |         value = [{"value": "fake_name", "op": "replace", "path": "/name"}, | ||||||
|                  '{"value": "fake_value", "op": "add", ' |                  {"value": "fake_value", "op": "add", | ||||||
|                  '"path": "/new_property"}]') |                   "path": "/instance_uuid"}] | ||||||
|         fake_img = sot.to_dict() |  | ||||||
|         fake_img['name'] = 'fake_name' |  | ||||||
|         fake_img['new_property'] = 'fake_value' |  | ||||||
|  |  | ||||||
|         sot.commit(self.sess, **fake_img) |         sot.name = 'fake_name' | ||||||
|  |         sot.instance_uuid = 'fake_value' | ||||||
|  |         sot.commit(self.sess) | ||||||
|         url = 'images/' + IDENTIFIER |         url = 'images/' + IDENTIFIER | ||||||
|         self.sess.patch.assert_called_once() |         self.sess.patch.assert_called_once() | ||||||
|         call = self.sess.patch.call_args |         call = self.sess.patch.call_args | ||||||
|         call_args, call_kwargs = call |         call_args, call_kwargs = call | ||||||
|         self.assertEqual(url, call_args[0]) |         self.assertEqual(url, call_args[0]) | ||||||
|         self.assertEqual( |         self.assertEqual( | ||||||
|             sorted(json.loads(value), key=operator.itemgetter('value')), |             sorted(value, key=operator.itemgetter('value')), | ||||||
|             sorted( |             sorted(call_kwargs['json'], key=operator.itemgetter('value'))) | ||||||
|                 json.loads(call_kwargs['data']), |  | ||||||
|                 key=operator.itemgetter('value'))) |  | ||||||
|   | |||||||
							
								
								
									
										5
									
								
								releasenotes/notes/image-update-76bd3bf24c1c1380.yaml
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										5
									
								
								releasenotes/notes/image-update-76bd3bf24c1c1380.yaml
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,5 @@ | |||||||
|  | --- | ||||||
|  | upgrade: | ||||||
|  |   - | | ||||||
|  |     When using the Image API, it is no longer possible to set arbitrary | ||||||
|  |     properties, not known to the SDK, via ``image.update_image`` API. | ||||||
		Reference in New Issue
	
	Block a user
	 Dmitry Tantsur
					Dmitry Tantsur