)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":360,"name":"Dan Prince","email":"dprince@redhat.com","username":"dan-prince"},"change_message_id":"5c5c4729ad355c54f3f20bfe93099285cc7253de","unresolved":false,"context_lines":[{"line_number":1864,"context_line":"            return instance_types.delete_instance_type_info(sys_meta,"},{"line_number":1865,"context_line":"                                                            \u0027old_\u0027, \u0027new_\u0027)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"        return new_instance_type is not None and sys_meta or None"},{"line_number":1868,"context_line":""},{"line_number":1869,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":1870,"context_line":"    @wrap_instance_event"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2F%2FWQ%3D","line":1867,"updated":"2013-03-04 14:30:07.000000000","message":"This last return statement confuses me on first glance. I actually think having this function just cleanup the system metadata (which we could pass to it instead of instance) might make things a bit cleaner... then let the calling functions decide whether or not an instance update is required.\n\nAs is _cleanup_stored_instance_types just confuses me a bit.","commit_id":"c748edb59c127734bb682bd0443cb4a44dc7e8f6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"20f2d46102056a1366cee1e3f54c7a865594fe90","unresolved":false,"context_lines":[{"line_number":1864,"context_line":"            return instance_types.delete_instance_type_info(sys_meta,"},{"line_number":1865,"context_line":"                                                            \u0027old_\u0027, \u0027new_\u0027)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"        return new_instance_type is not None and sys_meta or None"},{"line_number":1868,"context_line":""},{"line_number":1869,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":1870,"context_line":"    @wrap_instance_event"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2F%2FOk%3D","line":1867,"in_reply_to":"AAAAN3%2F%2F%2FQI%3D","updated":"2013-03-04 15:45:34.000000000","message":"It was always calling instance_update, but unnecessarily in the migrate case. I just wanted to avoid making another call out to conductor for a db write for no reason in the migrate case... :)","commit_id":"c748edb59c127734bb682bd0443cb4a44dc7e8f6"},{"author":{"_account_id":360,"name":"Dan Prince","email":"dprince@redhat.com","username":"dan-prince"},"change_message_id":"037b8425701c2e506a039791c1eb4045c3eaeac3","unresolved":false,"context_lines":[{"line_number":1864,"context_line":"            return instance_types.delete_instance_type_info(sys_meta,"},{"line_number":1865,"context_line":"                                                            \u0027old_\u0027, \u0027new_\u0027)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"        return new_instance_type is not None and sys_meta or None"},{"line_number":1868,"context_line":""},{"line_number":1869,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":1870,"context_line":"    @wrap_instance_event"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2F%2FQI%3D","line":1867,"in_reply_to":"AAAAN3%2F%2F%2FS4%3D","updated":"2013-03-04 15:21:44.000000000","message":"No. Not advocating we replicate the logic here... I mean _finish_resize is still calling update_instance always... and does it really hurt to call _update_instance always in confirm_resize as well. It was before right?\n\nTo me if we just have _cleanup_stored_instance_types do the cleanup and not have it conditionally return sys_meta (so we can optimize our save calls?) this would read cleaner.","commit_id":"c748edb59c127734bb682bd0443cb4a44dc7e8f6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c13d0216321effa483c933a31350c3757c0603c","unresolved":false,"context_lines":[{"line_number":1864,"context_line":"            return instance_types.delete_instance_type_info(sys_meta,"},{"line_number":1865,"context_line":"                                                            \u0027old_\u0027, \u0027new_\u0027)"},{"line_number":1866,"context_line":""},{"line_number":1867,"context_line":"        return new_instance_type is not None and sys_meta or None"},{"line_number":1868,"context_line":""},{"line_number":1869,"context_line":"    @exception.wrap_exception(notifier\u003dnotifier, publisher_id\u003dpublisher_id())"},{"line_number":1870,"context_line":"    @wrap_instance_event"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2F%2FS4%3D","line":1867,"in_reply_to":"AAAAN3%2F%2F%2FWQ%3D","updated":"2013-03-04 14:59:44.000000000","message":"So you think I should replicate all the logic at the call sites as well? The \"if new_instance_type is not None\" is fine, of course, but the whole point of the cleanup function was to encapsulate the behavior of us conditionally unstashing the old_ and new_ instance_type items based on whether we were changing flavors. I guess I don\u0027t see the point of putting it in a helper function otherwise.\n\nWhat if I changed the last statement to a clearer if..else block?","commit_id":"c748edb59c127734bb682bd0443cb4a44dc7e8f6"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"1160f27d6d25882ac5d1498d258a15839bb17d00","unresolved":false,"context_lines":[{"line_number":1875,"context_line":"        Returns system_metadata if an instance_update is needed, else None."},{"line_number":1876,"context_line":"        \"\"\""},{"line_number":1877,"context_line":"        metadata_changed \u003d False"},{"line_number":1878,"context_line":"        sys_meta \u003d utils.metadata_to_dict(instance[\u0027system_metadata\u0027])"},{"line_number":1879,"context_line":"        if new_instance_type is not None:"},{"line_number":1880,"context_line":"            sys_meta \u003d instance_types.save_instance_type_info("},{"line_number":1881,"context_line":"                sys_meta, new_instance_type)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAN3%2F%2F8Tw%3D","line":1878,"updated":"2013-03-05 18:21:17.000000000","message":"If you\u0027re reverting a migration (e.g. non-resize) will new_instance_type \u003d\u003d {} here?  Would you end up saving an empty dict?\n\nI don\u0027t see where the \u0027old_\u0027 prefix gets set in the non-resize case.","commit_id":"7e2cfc4480305af5acdafdbc97c2efbf2cfa6f06"},{"author":{"_account_id":6983,"name":"YangLei","email":"iamhappg@aliyun.com","username":"YangLei"},"change_message_id":"7a0ec604ee7afef099d6517c8cf849c454470dd7","unresolved":false,"context_lines":[{"line_number":1907,"context_line":"                                                   reservations):"},{"line_number":1908,"context_line":"            # NOTE(danms): delete stashed old/new instance_type information"},{"line_number":1909,"context_line":"            sys_meta \u003d self._cleanup_stored_instance_types(migration, instance)"},{"line_number":1910,"context_line":"            if sys_meta is not None:"},{"line_number":1911,"context_line":"                self._instance_update(context, instance[\u0027uuid\u0027],"},{"line_number":1912,"context_line":"                                      system_metadata\u003dsys_meta)"},{"line_number":1913,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAN3%2F%2F7Rw%3D","line":1910,"updated":"2013-03-06 06:09:23.000000000","message":"For migrate, _cleanup_stored_instance_types() just return None and do nothing here, because migration[\u0027old_instance_type_id\u0027] \u003d\u003d migration[\u0027new_instance_type_id\u0027]. And JUST prefix\u003d\u0027old_\u0027 info and prefix\u003d\u0027\u0027 info are in the sys_meta, because the old_instance_type_id \u003d\u003d new_instance_type_id, no prefix\u003d\u0027old_\u0027 info are saved into sys_meta in function _finish_resize()","commit_id":"7e2cfc4480305af5acdafdbc97c2efbf2cfa6f06"},{"author":{"_account_id":6983,"name":"YangLei","email":"iamhappg@aliyun.com","username":"YangLei"},"change_message_id":"7a0ec604ee7afef099d6517c8cf849c454470dd7","unresolved":false,"context_lines":[{"line_number":2018,"context_line":"            self._notify_about_instance_usage("},{"line_number":2019,"context_line":"                    context, instance, \"resize.revert.start\")"},{"line_number":2020,"context_line":""},{"line_number":2021,"context_line":"            instance_type \u003d instance_types.extract_instance_type(instance,"},{"line_number":2022,"context_line":"                                                                 prefix\u003d\u0027old_\u0027)"},{"line_number":2023,"context_line":"            sys_meta \u003d self._cleanup_stored_instance_types(migration,"},{"line_number":2024,"context_line":"                                                           instance,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAN3%2F%2F7SY%3D","line":2021,"updated":"2013-03-06 06:09:23.000000000","message":"For migrate, extract_instance_type() will get the error (e.g. KeyError: \u0027old_instance_type_memory_mb\u0027) here, when run \"resize-revert\". Because the old_instance_type_id \u003d\u003d new_instance_type_id, no prefix\u003d\u0027old_\u0027 info are saved into sys_meta in function _finish_resize()","commit_id":"7e2cfc4480305af5acdafdbc97c2efbf2cfa6f06"},{"author":{"_account_id":6983,"name":"YangLei","email":"iamhappg@aliyun.com","username":"YangLei"},"change_message_id":"edb96fea972690dba8fe1d0ee43ba7a7595f38c1","unresolved":false,"context_lines":[{"line_number":2013,"context_line":"                    context, instance, \"resize.revert.start\")"},{"line_number":2014,"context_line":""},{"line_number":2015,"context_line":"            instance_type \u003d instance_types.extract_instance_type(instance,"},{"line_number":2016,"context_line":"                                                                 \u0027old_\u0027)"},{"line_number":2017,"context_line":"            sys_meta \u003d self._cleanup_stored_instance_types(migration,"},{"line_number":2018,"context_line":"                                                           instance,"},{"line_number":2019,"context_line":"                                                           instance_type)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2F5tk%3D","line":2016,"updated":"2013-03-07 02:03:29.000000000","message":"extract_instance_type try to get the \u0027old_\u0027 info from sys_meta, but will get exception. Because the \u0027old_\u0027 info do not exist at all.\nThe prefix\u003d\u0027old_\u0027 info are saved in sys_meta, ONLY when old_instance_type_id !\u003d new_instance_type_id in function _finish_resize(). For migration, old_instance_type_id \u003d\u003d new_instance_type_id, so NO prefix\u003d\u0027old_\u0027 info will be saved.","commit_id":"2d1730297a12328da1e12d49af696a63af4e5121"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d88559fce7b8d2206457c5909306bb76c96fcee7","unresolved":false,"context_lines":[{"line_number":2013,"context_line":"                    context, instance, \"resize.revert.start\")"},{"line_number":2014,"context_line":""},{"line_number":2015,"context_line":"            instance_type \u003d instance_types.extract_instance_type(instance,"},{"line_number":2016,"context_line":"                                                                 \u0027old_\u0027)"},{"line_number":2017,"context_line":"            sys_meta \u003d self._cleanup_stored_instance_types(migration,"},{"line_number":2018,"context_line":"                                                           instance,"},{"line_number":2019,"context_line":"                                                           instance_type)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAN3%2F%2F5mQ%3D","line":2016,"in_reply_to":"AAAAN3%2F%2F5tk%3D","updated":"2013-03-07 03:56:51.000000000","message":"Right, duh, fix coming soon.","commit_id":"2d1730297a12328da1e12d49af696a63af4e5121"}],"nova/tests/compute/test_compute.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"0d99fef05c573d102911956500d35c184d62ae0a","unresolved":false,"context_lines":[{"line_number":2601,"context_line":"                                                          revert)"},{"line_number":2602,"context_line":"        self.assertEqual(res,"},{"line_number":2603,"context_line":"                         (sys_meta,"},{"line_number":2604,"context_line":"                          {\u0027instance_type_id\u0027: revert and old or new}))"},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"    def test_cleanup_stored_instance_types_for_resize(self):"},{"line_number":2607,"context_line":"        self._test_cleanup_stored_instance_types(\u00271\u0027, \u00272\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAN3%2F%2F4rI%3D","line":2604,"updated":"2013-03-07 16:59:19.000000000","message":"Dan, can we please change this to? makes my brain parse it better :)\n\n    {\u0027instance_type_id\u0027: old if revert else new}","commit_id":"5f3fa391ed499750ad68ad5b000b4e2e0a86978e"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"0b875c9a869b3b8269917e362e0a1f430ec818d0","unresolved":false,"context_lines":[{"line_number":2601,"context_line":"                                                          revert)"},{"line_number":2602,"context_line":"        self.assertEqual(res,"},{"line_number":2603,"context_line":"                         (sys_meta,"},{"line_number":2604,"context_line":"                          {\u0027instance_type_id\u0027: revert and old or new}))"},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"    def test_cleanup_stored_instance_types_for_resize(self):"},{"line_number":2607,"context_line":"        self._test_cleanup_stored_instance_types(\u00271\u0027, \u00272\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAN3%2F%2F2hI%3D","line":2604,"in_reply_to":"AAAAN3%2F%2F4qA%3D","updated":"2013-03-08 20:17:53.000000000","message":"I personally prefer the inline \u0027if\u0027 style, and that\u0027s despite being a C developer for the past, oh, 20 or so years.  The reason for my preference is that the inline \u0027if\u0027 style is idiomatic for Python, and was introduced relatively recently despite the availability of the and/or syntax, suggesting that more people found inline \u0027if\u0027 readable than the and/or syntax.\n\nThat said, I don\u0027t feel strongly enough to downcheck this change just for this…","commit_id":"5f3fa391ed499750ad68ad5b000b4e2e0a86978e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"872bbcd1211433729a52d56fdaa10aedb9880097","unresolved":false,"context_lines":[{"line_number":2601,"context_line":"                                                          revert)"},{"line_number":2602,"context_line":"        self.assertEqual(res,"},{"line_number":2603,"context_line":"                         (sys_meta,"},{"line_number":2604,"context_line":"                          {\u0027instance_type_id\u0027: revert and old or new}))"},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"    def test_cleanup_stored_instance_types_for_resize(self):"},{"line_number":2607,"context_line":"        self._test_cleanup_stored_instance_types(\u00271\u0027, \u00272\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAN3%2F%2F4qA%3D","line":2604,"in_reply_to":"AAAAN3%2F%2F4rI%3D","updated":"2013-03-07 17:07:31.000000000","message":"I\u0027d rather not. The and/or notation puts the condition first, which makes more sense to me and folks used to the C ternary operator, I think, since it\u0027s the same pattern.\n\nFWIW, I think there are more examples of this method than the one using inline \u0027if\u0027 in the openstack code.","commit_id":"5f3fa391ed499750ad68ad5b000b4e2e0a86978e"}]}