)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"68f31fca90b785fbd81d4179d0190be690b1a8dc","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"I realise that there are no tests submitted with this patch,"},{"line_number":15,"context_line":"they will follow however I would appreciate comments on this"},{"line_number":16,"context_line":"approach."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Patchset 2 fixes the pep8 formatting issues"},{"line_number":19,"context_line":"Patchset 3 fixes incorrect indents"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"AAAAO3%2F%2F%2BRY%3D","line":16,"updated":"2013-05-15 20:23:31.000000000","message":"I want tests in the patch before I will +2.  However, I do notice that your updates below mention tests; is this paragraph still describing the reality of this patch?","commit_id":"2a0a9cd44632a293fe6104d882fc515877be8b88"}],"nova/compute/api.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"68f31fca90b785fbd81d4179d0190be690b1a8dc","unresolved":false,"context_lines":[{"line_number":558,"context_line":"                                volume \u003d self.volume_api.get(context,"},{"line_number":559,"context_line":"                                        volume_id)"},{"line_number":560,"context_line":"                                for key, value in \\"},{"line_number":561,"context_line":"                                        self._get_volume_metadata_from_cinder("},{"line_number":562,"context_line":"                                        context, volume).iteritems():"},{"line_number":563,"context_line":"                                    props[key] \u003d value"},{"line_number":564,"context_line":"                            except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAO3%2F%2F%2BQk%3D","line":561,"updated":"2013-05-15 20:23:31.000000000","message":"I would slightly prefer if you reformatted this as:\n\n    vol_metadata \u003d self._get_volume_metadata_from_cinder(…)\n    for key, value in vol_metadata.iteritems():\n        …\n\nAnd actually, I do not understand why you use this idiom at all; surely you can simply do:\n\n    vol_metadata \u003d self._get_volume_metadata_from_cinder(…)\n    props.update(vol_metadata)\n\nAlso, I have to point out that _get_volume_metadata_from_cinder() can return None, so you need to have some guard here…","commit_id":"2a0a9cd44632a293fe6104d882fc515877be8b88"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"68f31fca90b785fbd81d4179d0190be690b1a8dc","unresolved":false,"context_lines":[{"line_number":834,"context_line":"                    raise exception.InvalidBDMSnapshot(id\u003dsnapshot_id)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"    def _get_volume_metadata_from_cinder(self, context, volume):"},{"line_number":837,"context_line":"        # before we retireve the metadata, let\u0027s check and see if it is there:"},{"line_number":838,"context_line":"        if volume.get(\u0027volume_image_metadata\u0027) !\u003d \"\":"},{"line_number":839,"context_line":"            LOG.debug(\"Volume Metadata: %s\" %"},{"line_number":840,"context_line":"                    volume.get(\u0027volume_image_metadata\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAO3%2F%2F%2BQg%3D","line":837,"updated":"2013-05-15 20:23:31.000000000","message":"s/retireve/retrieve/","commit_id":"2a0a9cd44632a293fe6104d882fc515877be8b88"},{"author":{"_account_id":1528,"name":"Stanislaw Pitucha","email":"viraptor@gmail.com","username":"stanislaw-pitucha"},"change_message_id":"5a827be301deaa6cb7fd8ab0736c490b7fd396ad","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#vim: tabstop\u003d4 shiftwidth\u003d4 softtabstop\u003d4"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"# Copyright 2010 United States Government as represented by the"},{"line_number":4,"context_line":"# Administrator of the National Aeronautics and Space Administration."}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAO3%2F%2F8gs%3D","line":1,"updated":"2013-05-16 14:37:25.000000000","message":"accidental delete? there should be a whitespace before \"vim:\"","commit_id":"6bd47d083d95fea4307c5b8876de7d61b42e06bd"},{"author":{"_account_id":1528,"name":"Stanislaw Pitucha","email":"viraptor@gmail.com","username":"stanislaw-pitucha"},"change_message_id":"5a827be301deaa6cb7fd8ab0736c490b7fd396ad","unresolved":false,"context_lines":[{"line_number":547,"context_line":"                if image[\u0027status\u0027] !\u003d \u0027active\u0027:"},{"line_number":548,"context_line":"                    raise exception.ImageNotActive(image_id\u003dimage_id)"},{"line_number":549,"context_line":"            else:"},{"line_number":550,"context_line":"                image \u003d {}"},{"line_number":551,"context_line":"                # get the volume metadata and apply it to the instance"},{"line_number":552,"context_line":"                props \u003d {}"},{"line_number":553,"context_line":"                for bdm in block_device_mapping:"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAO3%2F%2F8gk%3D","line":550,"updated":"2013-05-16 14:37:25.000000000","message":"image is overwritten below - this line seems redundant","commit_id":"6bd47d083d95fea4307c5b8876de7d61b42e06bd"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"ffd300dbff85683f83ecda519c4ba5d0eb028fdf","unresolved":false,"context_lines":[{"line_number":597,"context_line":"            # exception instead of a OverlimitException."},{"line_number":598,"context_line":"            self._check_metadata_properties_quota(context, metadata)"},{"line_number":599,"context_line":""},{"line_number":600,"context_line":"            if image_href:"},{"line_number":601,"context_line":"                image_id, image \u003d self._get_image(context, image_href)"},{"line_number":602,"context_line":"                self._checks_for_create_and_rebuild(context, image_id, image,"},{"line_number":603,"context_line":"                    instance_type, metadata, injected_files)"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAAPH%2F%2Fjw0%3D","line":600,"updated":"2013-05-30 15:27:21.000000000","message":"Maybe this logic, or some portion of it, should be pulled out into another function?  The indentation is starting to get rather deep…","commit_id":"4e8b570b1e1f3edc8df650bc6021b83ee899b44d"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"495548e34c81ff9da6a82a43d287d7d0e63084f4","unresolved":false,"context_lines":[{"line_number":600,"context_line":"        try:"},{"line_number":601,"context_line":"            instances \u003d []"},{"line_number":602,"context_line":"            instance_uuids \u003d []"},{"line_number":603,"context_line":""},{"line_number":604,"context_line":"            if image_href:"},{"line_number":605,"context_line":"                image_id, image \u003d self._get_image(context, image_href)"},{"line_number":606,"context_line":"                self._checks_for_create_and_rebuild(context, image_id, image,"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPH%2F%2FfuE%3D","line":603,"updated":"2013-05-31 21:05:13.000000000","message":"I would wrap this if/else block into a function call to make it easier to reason about - what side effects happen in each case. For instance - is image_id available to the rest of the function?","commit_id":"ed73a1d40a83f83e5cb34fbdbc1de1cd03a0c448"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"09b249991f477fa5a4b53442fb749e232753ac6a","unresolved":false,"context_lines":[{"line_number":620,"context_line":"                                        context, volume)"},{"line_number":621,"context_line":"                                if vol_metadata is not None:"},{"line_number":622,"context_line":"                                    props.update(vol_metadata)"},{"line_number":623,"context_line":"                            except Exception:"},{"line_number":624,"context_line":"                                raise exception.InvalidBDMVolume(id\u003dvolume_id)"},{"line_number":625,"context_line":"                image \u003d {\u0027properties\u0027: props}"},{"line_number":626,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPH%2F%2Fd3I%3D","line":623,"updated":"2013-06-03 09:26:54.000000000","message":"This try/except only needs to wrap line 616","commit_id":"ed73a1d40a83f83e5cb34fbdbc1de1cd03a0c448"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"495548e34c81ff9da6a82a43d287d7d0e63084f4","unresolved":false,"context_lines":[{"line_number":881,"context_line":"                except Exception:"},{"line_number":882,"context_line":"                    raise exception.InvalidBDMSnapshot(id\u003dsnapshot_id)"},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"    def _get_volume_metadata_from_cinder(self, context, volume):"},{"line_number":885,"context_line":"        # before we retrieve the metadata, let\u0027s check and see if it is there:"},{"line_number":886,"context_line":"        metadata \u003d volume.get(\u0027volume_image_metadata\u0027)"},{"line_number":887,"context_line":"        if metadata:"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPH%2F%2Ffuc%3D","line":884,"updated":"2013-05-31 21:05:13.000000000","message":"A docstring would be good, if you don\u0027t inline this.","commit_id":"ed73a1d40a83f83e5cb34fbdbc1de1cd03a0c448"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"495548e34c81ff9da6a82a43d287d7d0e63084f4","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # before we retrieve the metadata, let\u0027s check and see if it is there:"},{"line_number":886,"context_line":"        metadata \u003d volume.get(\u0027volume_image_metadata\u0027)"},{"line_number":887,"context_line":"        if metadata:"},{"line_number":888,"context_line":"            return metadata"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"    def _populate_instance_for_bdm(self, context, instance, instance_type,"},{"line_number":891,"context_line":"            image, block_device_mapping):"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPH%2F%2Ffug%3D","line":888,"updated":"2013-05-31 21:05:13.000000000","message":"This is a little odd - at a guess you are translating {} into None ?\n\nI would write it as\nreturn volume.get(\u0027volume_image_metadata\u0027) or None\n\nwhich btw is one line and perhaps best inlined ?","commit_id":"ed73a1d40a83f83e5cb34fbdbc1de1cd03a0c448"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"09b249991f477fa5a4b53442fb749e232753ac6a","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # before we retrieve the metadata, let\u0027s check and see if it is there:"},{"line_number":886,"context_line":"        metadata \u003d volume.get(\u0027volume_image_metadata\u0027)"},{"line_number":887,"context_line":"        if metadata:"},{"line_number":888,"context_line":"            return metadata"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"    def _populate_instance_for_bdm(self, context, instance, instance_type,"},{"line_number":891,"context_line":"            image, block_device_mapping):"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAPH%2F%2Fd30%3D","line":888,"in_reply_to":"AAAAPH%2F%2Ffug%3D","updated":"2013-06-03 09:26:54.000000000","message":"So this method can be written as\n\n def _get_volume_metadata_from_cinder(self, context, volume):\n     return volume.get(\u0027volume_image_metadata\u0027)\n\nThen I don\u0027t see why we need this method.","commit_id":"ed73a1d40a83f83e5cb34fbdbc1de1cd03a0c448"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4ef26831e00688c32fed3283cca591873ac483cb","unresolved":false,"context_lines":[{"line_number":714,"context_line":"            \u0027instance_uuids\u0027: instance_uuids,"},{"line_number":715,"context_line":"            \u0027block_device_mapping\u0027: block_device_mapping,"},{"line_number":716,"context_line":"            \u0027security_group\u0027: security_groups,"},{"line_number":717,"context_line":"            \u0027metadata\u0027: metadata,"},{"line_number":718,"context_line":"        }"},{"line_number":719,"context_line":""},{"line_number":720,"context_line":"        return (instances, request_spec, filter_properties)"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAPX%2F%2F5ls%3D","line":717,"updated":"2013-06-06 14:00:36.000000000","message":"I\u0027m not sure what this is doing here.  But if it needs to be passed then (after rebase) you\u0027ll need to add it to the self.compute_task_api.build_instances() call as a new parameter.  Request_spec is built in the conductor manager now and contains just the info necessary for the scheduler.  Well, it will soon since schedule_run_instance is being deprecated.","commit_id":"300c58c81cd28bcab9a1a8396f15b24f492cb4c6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4ef26831e00688c32fed3283cca591873ac483cb","unresolved":false,"context_lines":[{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        :return: A dict with image properties."},{"line_number":731,"context_line":"        \"\"\""},{"line_number":732,"context_line":"        if image_href:"},{"line_number":733,"context_line":"            image_id, image \u003d self._get_image(context, image_href)"},{"line_number":734,"context_line":"            self._checks_for_create_and_rebuild(context, image_id, image,"},{"line_number":735,"context_line":"                instance_type, metadata, injected_files)"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAPX%2F%2F5lY%3D","line":732,"updated":"2013-06-06 14:00:36.000000000","message":"I would move this check out of here, and do something like the following in _create_instance(after rebase):\n\nif image_href:\n    _, _ \u003d self._get_image()\nelse:\n   image_id \u003d None\n    _ \u003d self._get_volume()\n\nIf renaming image makes sense then do so.","commit_id":"300c58c81cd28bcab9a1a8396f15b24f492cb4c6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"88500d6b2688f8fa49c729ddca8544bb494c0cd6","unresolved":false,"context_lines":[{"line_number":729,"context_line":""},{"line_number":730,"context_line":"        :return: A dict with image properties."},{"line_number":731,"context_line":"        \"\"\""},{"line_number":732,"context_line":"        if image_href:"},{"line_number":733,"context_line":"            image_id, image \u003d self._get_image(context, image_href)"},{"line_number":734,"context_line":"            self._checks_for_create_and_rebuild(context, image_id, image,"},{"line_number":735,"context_line":"                instance_type, metadata, injected_files)"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAPX%2F%2F5Fg%3D","line":732,"in_reply_to":"AAAAPX%2F%2F5lY%3D","updated":"2013-06-06 16:28:58.000000000","message":"Well, don\u0027t use a literal \"_\" in the above, for obvious reasons…","commit_id":"300c58c81cd28bcab9a1a8396f15b24f492cb4c6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9f48df98e68f05e2478ea2fa5d63f91b27044b69","unresolved":false,"context_lines":[{"line_number":743,"context_line":"                        try:"},{"line_number":744,"context_line":"                            volume \u003d self.volume_api.get(context,"},{"line_number":745,"context_line":"                                    volume_id)"},{"line_number":746,"context_line":"                            vol_metadata \u003d \\"},{"line_number":747,"context_line":"                                    self._get_volume_metadata_from_cinder("},{"line_number":748,"context_line":"                                    context, volume)"},{"line_number":749,"context_line":"                            if vol_metadata is not None:"}],"source_content_type":"text/x-python","patch_set":20,"id":"AAAAPX%2F%2F7r4%3D","line":746,"updated":"2013-06-05 19:37:45.000000000","message":"Could you break this up a bit and/or reduce the nesting? When you start having to break lines early like that, it usually means you\u0027re wayyy too deep :)\n\nI think you can pull at least one level out with some logic compression, but a helper function may be appropriate here as well.","commit_id":"300c58c81cd28bcab9a1a8396f15b24f492cb4c6"},{"author":{"_account_id":1528,"name":"Stanislaw Pitucha","email":"viraptor@gmail.com","username":"stanislaw-pitucha"},"change_message_id":"8db67ef7bc96557425864cabf3a9f5d896881756","unresolved":false,"context_lines":[{"line_number":692,"context_line":"        \"\"\"If we are booting from a volume, we need to get the"},{"line_number":693,"context_line":"        volume details from Cinder and make sure we pass the"},{"line_number":694,"context_line":"        metadata back accordingly\"\"\""},{"line_number":695,"context_line":"        if not block_device_mapping:"},{"line_number":696,"context_line":"            return None, {}"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"        for bdm in block_device_mapping:"}],"source_content_type":"text/x-python","patch_set":22,"id":"AAAAPX%2F%2F2e8%3D","line":695,"updated":"2013-06-07 15:28:57.000000000","message":"This check is not needed - block_device_mapping is already checked for being truthy in line 738","commit_id":"a3165770c7b40307490aef7b2517c6aeb8aaecc7"},{"author":{"_account_id":1528,"name":"Stanislaw Pitucha","email":"viraptor@gmail.com","username":"stanislaw-pitucha"},"change_message_id":"8db67ef7bc96557425864cabf3a9f5d896881756","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        if image_href:"},{"line_number":736,"context_line":"            image_id, image \u003d self._get_image(context, image_href)"},{"line_number":737,"context_line":"        else:"},{"line_number":738,"context_line":"            if block_device_mapping is not []:"},{"line_number":739,"context_line":"                image_id, image \u003d self._get_volume(context,"},{"line_number":740,"context_line":"                        block_device_mapping)"},{"line_number":741,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"AAAAPX%2F%2F2eQ%3D","line":738,"updated":"2013-06-07 15:28:57.000000000","message":"That\u0027s almost never false. `[] is [] \u003d\u003d False` The check should be for `... !\u003d []` or just `if block_device_mapping:`","commit_id":"a3165770c7b40307490aef7b2517c6aeb8aaecc7"},{"author":{"_account_id":6896,"name":"Loganathan Parthipan","email":"parthi@hpe.com","username":"parthipan"},"change_message_id":"94259331f5b09b778d0da2f881e05880f749319d","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            image_id, image \u003d self._get_image(context, image_href)"},{"line_number":737,"context_line":"        else:"},{"line_number":738,"context_line":"            if block_device_mapping is not []:"},{"line_number":739,"context_line":"                image_id, image \u003d self._get_volume(context,"},{"line_number":740,"context_line":"                        block_device_mapping)"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"        handle_az \u003d self._handle_availability_zone"}],"source_content_type":"text/x-python","patch_set":22,"id":"AAAAPX%2F%2F2f4%3D","line":739,"updated":"2013-06-07 15:21:03.000000000","message":"_get_volume returns volume_id. You might not want to have it as the image_id as it will break novaclient when listing vm details","commit_id":"a3165770c7b40307490aef7b2517c6aeb8aaecc7"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"b4c4ddfed7d63129469da933ac5ff5b939016fd9","unresolved":false,"context_lines":[{"line_number":691,"context_line":"    def _get_volume(self, context, block_device_mapping):"},{"line_number":692,"context_line":"        \"\"\"If we are booting from a volume, we need to get the"},{"line_number":693,"context_line":"        volume details from Cinder and make sure we pass the"},{"line_number":694,"context_line":"        metadata back accordingly\"\"\""},{"line_number":695,"context_line":"        if not block_device_mapping:"},{"line_number":696,"context_line":"            return {}"},{"line_number":697,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"AAAAPX%2F%2F2Y0%3D","line":694,"updated":"2013-06-07 15:59:35.000000000","message":"Nit: I prefer the closing triple-quotes to be on a line by itself.  Also, this sentence should be ended by a period.","commit_id":"9fcf4e887fb0422be09f1f77a9d9e1b16196d79f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"83a981cb3d52c4f58c139631650f484f98722bb0","unresolved":false,"context_lines":[{"line_number":539,"context_line":"                                       files_to_inject):"},{"line_number":540,"context_line":"        self._check_metadata_properties_quota(context, metadata)"},{"line_number":541,"context_line":"        self._check_injected_file_quota(context, files_to_inject)"},{"line_number":542,"context_line":"        if image_id is not None:"},{"line_number":543,"context_line":"            self._check_requested_image(context, image_id,"},{"line_number":544,"context_line":"                    image, instance_type)"},{"line_number":545,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"AAAAPX%2F%2FzDc%3D","line":542,"updated":"2013-06-10 13:53:25.000000000","message":"This check is unnecessary, since it\u0027s the first check in _check_requested_image, but it doesn\u0027t hurt anything.","commit_id":"e67641da51610a419ec8cf6562583d14a2b0fbd7"}]}