)]}'
{"nova/objects/instance.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"9136b5a03cab2134e11ef26b2dde55d2903a0205","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        # NOTE(danms): We can be called with a dict instead of a"},{"line_number":336,"context_line":"        # SQLAlchemy object, so we have to be careful here"},{"line_number":337,"context_line":"        have_extra \u003d \u0027extra\u0027 in ("},{"line_number":338,"context_line":"            db_inst.__dict__ if hasattr(db_inst, \u0027__dict__\u0027) else db_inst"},{"line_number":339,"context_line":"        ) and db_inst[\u0027extra\u0027]"},{"line_number":340,"context_line":"        instance \u003d objects.Instance._load_arrays(context, instance,"},{"line_number":341,"context_line":"                                                 db_inst, expected_attrs)"},{"line_number":342,"context_line":"        instance \u003d objects.Instance._load_attribs(context, instance,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_d8bd0fd9","line":339,"range":{"start_line":337,"start_character":8,"end_line":339,"end_character":30},"updated":"2016-08-24 15:43:34.000000000","message":"I\u0027m afraid I prefer the original logic; this construction is somewhat opaque compared to the if/else tree.","commit_id":"241e1d06e934afa89509a36f356bf863ee0ee961"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"9136b5a03cab2134e11ef26b2dde55d2903a0205","unresolved":false,"context_lines":[{"line_number":342,"context_line":"        instance \u003d objects.Instance._load_attribs(context, instance,"},{"line_number":343,"context_line":"                                                  db_inst, expected_attrs, have_extra)"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"        instance.obj_reset_changes()"},{"line_number":346,"context_line":"        return instance"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_58f47f86","line":345,"updated":"2016-08-24 15:43:34.000000000","message":"I don\u0027t quite get the assignments on lines 340 and 342.  As far as I can see, these methods don\u0027t actually change the instance of Instance being referenced, they just alter some of its attributes.","commit_id":"241e1d06e934afa89509a36f356bf863ee0ee961"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"9136b5a03cab2134e11ef26b2dde55d2903a0205","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    @staticmethod"},{"line_number":349,"context_line":"    def _load_attribs(context, instance, db_inst, expected_attrs\u003dNone,"},{"line_number":350,"context_line":"                      have_extra\u003dNone):"},{"line_number":351,"context_line":"        if \u0027numa_topology\u0027 in expected_attrs:"},{"line_number":352,"context_line":"            instance.numa_topology \u003d None"},{"line_number":353,"context_line":"            if have_extra:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_38d0a3c9","line":350,"updated":"2016-08-24 15:43:34.000000000","message":"Is there a particular reason these methods are static methods?  I realize the original _from_db_object() method is written using the same style, but I figure it\u0027s worth asking whether these subordinate methods should be written that way.","commit_id":"241e1d06e934afa89509a36f356bf863ee0ee961"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"cab96be0623d7ec5eaafbc1d4e8fb4c823365256","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import contextlib"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from decorator import getfullargspec"},{"line_number":18,"context_line":"from oslo_config import cfg"},{"line_number":19,"context_line":"from oslo_db import exception as db_exc"},{"line_number":20,"context_line":"from oslo_log import log as logging"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_6cd32660","line":17,"updated":"2016-08-25 18:34:32.000000000","message":"1. Where is this import used?\n\n2. Only import modules, not objects; this makes it look like you\u0027re importing a function called getfullargspec()\n\n3. If you\u0027re really calling a function, check to see if oslo_utils may already have the functionality hiding away somewhere…","commit_id":"08908de1efce9b7ca2626b205d6d728d3f2ae368"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"cab96be0623d7ec5eaafbc1d4e8fb4c823365256","unresolved":false,"context_lines":[{"line_number":348,"context_line":"    def _load_attribs(context, instance, db_inst, expected_attrs\u003dNone,"},{"line_number":349,"context_line":"                      have_extra\u003dNone):"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        loads \u003d ((\"numa_topology\", True, True, True),"},{"line_number":352,"context_line":"                 (\"pci_request\", True, True, True),"},{"line_number":353,"context_line":"                 (\"device_metadata\", True, True, True),"},{"line_number":354,"context_line":"                 (\"vcpu_model\", True, True, True),"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_6c9e86b9","line":351,"updated":"2016-08-25 18:34:32.000000000","message":"Since this is a constant, I\u0027d like to see it part of the global scope.  That way, it doesn\u0027t have to be created each time this method is called.  (femtonit)","commit_id":"08908de1efce9b7ca2626b205d6d728d3f2ae368"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"b25d6e25e6ba39896de34373ca34e9959a78f1e5","unresolved":false,"context_lines":[{"line_number":392,"context_line":"                instance.info_cache._from_db_object(context,"},{"line_number":393,"context_line":"                                                    instance.info_cache,"},{"line_number":394,"context_line":"                                                    db_inst[\u0027info_cache\u0027])"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        if any([x in expected_attrs for x in (\u0027flavor\u0027,"},{"line_number":397,"context_line":"                                              \u0027old_flavor\u0027,"},{"line_number":398,"context_line":"                                              \u0027new_flavor\u0027)]):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa7ab95a_6ea04a44","side":"PARENT","line":395,"updated":"2016-08-29 09:15:58.000000000","message":"please, don\u0027t remove empty lines. They increase readability.","commit_id":"1423be58b5e3aa26c85296968146e9d6ed59455a"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"b25d6e25e6ba39896de34373ca34e9959a78f1e5","unresolved":false,"context_lines":[{"line_number":401,"context_line":"                instance.info_cache._from_db_object(context,"},{"line_number":402,"context_line":"                                                    instance.info_cache,"},{"line_number":403,"context_line":"                                                    db_inst[\u0027info_cache\u0027])"},{"line_number":404,"context_line":"        if any([x in expected_attrs for x in (\u0027flavor\u0027,"},{"line_number":405,"context_line":"                                              \u0027old_flavor\u0027, \u0027new_flavor\u0027)]"},{"line_number":406,"context_line":"               ) and ("},{"line_number":407,"context_line":"                    have_extra and db_inst[\u0027extra\u0027].get(\u0027flavor\u0027)):"},{"line_number":408,"context_line":"            instance._flavor_from_db(db_inst[\u0027extra\u0027][\u0027flavor\u0027])"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        if \u0027metadata\u0027 in expected_attrs:"},{"line_number":411,"context_line":"            instance[\u0027metadata\u0027] \u003d utils.instance_meta(db_inst)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa7ab95a_01b6af3c","line":408,"range":{"start_line":404,"start_character":8,"end_line":408,"end_character":64},"updated":"2016-08-29 09:15:58.000000000","message":"Maybe it\u0027s just me, but I would keep the original version, since it\u0027s more readable.","commit_id":"44883a363eef24a10b149dda16871f84996990e4"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"b25d6e25e6ba39896de34373ca34e9959a78f1e5","unresolved":false,"context_lines":[{"line_number":682,"context_line":"                            admin_state_reset)"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        self._extra_values_to_save \u003d {}"},{"line_number":685,"context_line":"        updates \u003d {}"},{"line_number":686,"context_line":"        changes \u003d self.obj_what_changed()"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"        updates \u003d self._object_construct_call_helper_method(context,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa7ab95a_df302204","line":685,"range":{"start_line":685,"start_character":8,"end_line":685,"end_character":20},"updated":"2016-08-29 09:15:58.000000000","message":"You don\u0027t need this dict, since you get it from the method (line 688).","commit_id":"44883a363eef24a10b149dda16871f84996990e4"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"b25d6e25e6ba39896de34373ca34e9959a78f1e5","unresolved":false,"context_lines":[{"line_number":754,"context_line":""},{"line_number":755,"context_line":"        self.obj_reset_changes()"},{"line_number":756,"context_line":""},{"line_number":757,"context_line":"    def _object_construct_call_helper_method(self, context, changes, updates):"},{"line_number":758,"context_line":"        for field in self.fields:"},{"line_number":759,"context_line":"            # NOTE(danms): For object fields, we construct and call a"},{"line_number":760,"context_line":"            # helper method like self._save_$attrname()"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa7ab95a_64ace9a0","line":757,"range":{"start_line":757,"start_character":8,"end_line":757,"end_character":45},"updated":"2016-08-29 09:15:58.000000000","message":"The method name is not accurate. Also, \"updates\" argument is not necessary, because it is an empty dictionary at the beginning (see line 686), so it can be created in this method.","commit_id":"44883a363eef24a10b149dda16871f84996990e4"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"1dad314ee34e9908fa001906b0ccd71f20a0bd6a","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                                                    instance.info_cache,"},{"line_number":403,"context_line":"                                                    db_inst[\u0027info_cache\u0027])"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"            if any([x in expected_attrs for x in (\u0027flavor\u0027,"},{"line_number":406,"context_line":"                                                      \u0027old_flavor\u0027,"},{"line_number":407,"context_line":"                                                      \u0027new_flavor\u0027)]):"},{"line_number":408,"context_line":"                if have_extra and db_inst[\u0027extra\u0027].get(\u0027flavor\u0027):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_27a7eabd","line":405,"range":{"start_line":405,"start_character":8,"end_line":405,"end_character":12},"updated":"2016-08-30 12:37:01.000000000","message":"You have just changed the logic of this function. Indentation does not match with old implementation.","commit_id":"c5f23c8f57e42278b7024757ee5530cb12d80e98"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"538ebb7a359ad3d65842951f31c393ee156826c0","unresolved":false,"context_lines":[{"line_number":403,"context_line":"                                                    db_inst[\u0027info_cache\u0027])"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"            if any([x in expected_attrs for x in (\u0027flavor\u0027,"},{"line_number":406,"context_line":"                                                      \u0027old_flavor\u0027,"},{"line_number":407,"context_line":"                                                      \u0027new_flavor\u0027)]):"},{"line_number":408,"context_line":"                if have_extra and db_inst[\u0027extra\u0027].get(\u0027flavor\u0027):"},{"line_number":409,"context_line":"                    instance._flavor_from_db(db_inst[\u0027extra\u0027][\u0027flavor\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_51c0652a","line":406,"range":{"start_line":406,"start_character":50,"end_line":406,"end_character":54},"updated":"2016-08-29 17:05:35.000000000","message":"If you\u0027re going to use this style, dedent this line so that \u0027old_flavor\u0027 lines up with \u0027flavor\u0027 from the preceding line.  (A similar dedent should occur on 407 as well, of course.)","commit_id":"c5f23c8f57e42278b7024757ee5530cb12d80e98"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"1dad314ee34e9908fa001906b0ccd71f20a0bd6a","unresolved":false,"context_lines":[{"line_number":749,"context_line":""},{"line_number":750,"context_line":"        self.obj_reset_changes()"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"    def _get_updates_dict(self, context, changes):"},{"line_number":753,"context_line":"        updates \u003d {}"},{"line_number":754,"context_line":"        for field in self.fields:"},{"line_number":755,"context_line":"            # NOTE(danms): For object fields, we construct and call a"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_0dd3e760","line":752,"range":{"start_line":752,"start_character":8,"end_line":752,"end_character":25},"updated":"2016-08-30 12:37:01.000000000","message":"Actually, this function, again, is doing more than providing \"update\" dictionary. It saving objects, which Instance contains. Update will be filled out with those fields, which are changed, and are not objects. Maybe \"_update_fields\" would be more sufficient.","commit_id":"c5f23c8f57e42278b7024757ee5530cb12d80e98"}]}