)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2017-04-17 12:10:27 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"placement: adds ProviderTree for nested resources"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We need a way of representing the hierarchical relationship between resource"},{"line_number":10,"context_line":"providers. This representation needs to be able to work with either UUID"},{"line_number":11,"context_line":"identifiers or unique string name identifiers due to the way that NUMA cell and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"df140735_65479f4d","line":8,"updated":"2017-05-30 20:01:47.000000000","message":"Reformat for line width (so I don\u0027t have to hscroll in gerrit).","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2017-04-17 12:10:27 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"placement: adds ProviderTree for nested resources"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We need a way of representing the hierarchical relationship between resource"},{"line_number":10,"context_line":"providers. This representation needs to be able to work with either UUID"},{"line_number":11,"context_line":"identifiers or unique string name identifiers due to the way that NUMA cell and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"df140735_cabe6f64","line":8,"in_reply_to":"df140735_65479f4d","updated":"2017-06-01 00:34:02.000000000","message":"Done","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":7,"context_line":"placement: adds ProviderTree for nested resources"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We need a way of representing the hierarchical relationship between resource"},{"line_number":10,"context_line":"providers. This representation needs to be able to work with either UUID"},{"line_number":11,"context_line":"identifiers or unique string name identifiers due to the way that NUMA cell and"},{"line_number":12,"context_line":"topology objects are constructed in the virt layer. It also needs to provide a"},{"line_number":13,"context_line":"way of updating the inventory of any of the providers in a tree and returning"},{"line_number":14,"context_line":"whether or not the inventory for the tree has changed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"df140735_6560bfcf","line":11,"range":{"start_line":10,"start_character":68,"end_line":11,"end_character":11},"updated":"2017-05-30 20:01:47.000000000","message":"redundant; suggest \"UUIDs\"","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":7,"context_line":"placement: adds ProviderTree for nested resources"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We need a way of representing the hierarchical relationship between resource"},{"line_number":10,"context_line":"providers. This representation needs to be able to work with either UUID"},{"line_number":11,"context_line":"identifiers or unique string name identifiers due to the way that NUMA cell and"},{"line_number":12,"context_line":"topology objects are constructed in the virt layer. It also needs to provide a"},{"line_number":13,"context_line":"way of updating the inventory of any of the providers in a tree and returning"},{"line_number":14,"context_line":"whether or not the inventory for the tree has changed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"df140735_aac1fbe6","line":11,"range":{"start_line":10,"start_character":68,"end_line":11,"end_character":11},"in_reply_to":"df140735_6560bfcf","updated":"2017-06-01 00:34:02.000000000","message":"Done","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"}],"nova/compute/provider_tree.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"843f3e95c0f7dae44a25285e3c37a2db53f04168","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def remove_child(self, provider):"},{"line_number":103,"context_line":"        with self.tree.lock:"},{"line_number":104,"context_line":"            if provider.uuid in self.children:"},{"line_number":105,"context_line":"                del self.children_name_map[provider.name]"},{"line_number":106,"context_line":"                del self.children[provider.uuid]"},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7a3c09a3_bf51f5da","line":104,"range":{"start_line":104,"start_character":12,"end_line":104,"end_character":46},"updated":"2017-01-19 14:00:18.000000000","message":"If it isn\u0027t in self.children, should it really just noop like this, or should it return an error?","commit_id":"3f48548e6adbe2ddc21ff2fa0651aaef487343fc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"14f1b8def8e958915b2c6090e5e9a194f2bea26d","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def remove_child(self, provider):"},{"line_number":103,"context_line":"        with self.tree.lock:"},{"line_number":104,"context_line":"            if provider.uuid in self.children:"},{"line_number":105,"context_line":"                del self.children_name_map[provider.name]"},{"line_number":106,"context_line":"                del self.children[provider.uuid]"},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_1f3829d3","line":104,"range":{"start_line":104,"start_character":12,"end_line":104,"end_character":46},"in_reply_to":"5a3905b3_bf3716e2","updated":"2017-01-19 18:08:52.000000000","message":"On second look, yeah, I did mean to include this :)\n\nAnd no, I didn\u0027t care to return an error because the _Provider object interface is internal. The ProviderTree.remove() method is the \"public\" interface and it raises a ValueError if remove() is called with a non-existing uuid or name.","commit_id":"3f48548e6adbe2ddc21ff2fa0651aaef487343fc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b132df6f546dee4a1ea96c941c71a7d152f6ff39","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def remove_child(self, provider):"},{"line_number":103,"context_line":"        with self.tree.lock:"},{"line_number":104,"context_line":"            if provider.uuid in self.children:"},{"line_number":105,"context_line":"                del self.children_name_map[provider.name]"},{"line_number":106,"context_line":"                del self.children[provider.uuid]"},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_bf3716e2","line":104,"range":{"start_line":104,"start_character":12,"end_line":104,"end_character":46},"in_reply_to":"7a3c09a3_bf51f5da","updated":"2017-01-19 17:47:09.000000000","message":"Actually, I didn\u0027t mean to include this method :) Leftover from some earlier PoC stuff... I\u0027ll remove this method and add it back when needed in later patches.","commit_id":"3f48548e6adbe2ddc21ff2fa0651aaef487343fc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":13,"context_line":"\"\"\"An object describing a tree of resource providers and their inventories."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This object is not stored in the Nova API or cell databases; rather, this"},{"line_number":16,"context_line":"object is constructed by and used by the scheduler report client to track state"},{"line_number":17,"context_line":"changes for resources on the hypervisor or baremetal node. As such, there are"},{"line_number":18,"context_line":"no remoteable methods nor is there any interaction with the nova.db modules."},{"line_number":19,"context_line":"\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_85b8732a","line":16,"range":{"start_line":16,"start_character":22,"end_line":16,"end_character":24},"updated":"2017-05-30 20:01:47.000000000","message":"strike","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":13,"context_line":"\"\"\"An object describing a tree of resource providers and their inventories."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This object is not stored in the Nova API or cell databases; rather, this"},{"line_number":16,"context_line":"object is constructed by and used by the scheduler report client to track state"},{"line_number":17,"context_line":"changes for resources on the hypervisor or baremetal node. As such, there are"},{"line_number":18,"context_line":"no remoteable methods nor is there any interaction with the nova.db modules."},{"line_number":19,"context_line":"\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_2aadeb30","line":16,"range":{"start_line":16,"start_character":22,"end_line":16,"end_character":24},"in_reply_to":"df140735_85b8732a","updated":"2017-06-01 00:34:02.000000000","message":"Done","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        new_rec \u003d new[key]"},{"line_number":37,"context_line":"        for rec_key, cur_val in cur_rec.items():"},{"line_number":38,"context_line":"            if rec_key not in new_rec:"},{"line_number":39,"context_line":"                # Deliberately don\u0027t want to compare missing keys in the"},{"line_number":40,"context_line":"                # inventory record. For instance, we will be passing in fields"},{"line_number":41,"context_line":"                # like allocation_ratio in the current dict but the resource"},{"line_number":42,"context_line":"                # tracker may only pass in the total field. We want to return"},{"line_number":43,"context_line":"                # that inventory didn\u0027t change when the total field values are"},{"line_number":44,"context_line":"                # the same even if the allocation_ratio field is missing from"},{"line_number":45,"context_line":"                # the new record."},{"line_number":46,"context_line":"                continue"},{"line_number":47,"context_line":"            if new_rec[rec_key] !\u003d cur_val:"},{"line_number":48,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_6518bf1c","line":45,"range":{"start_line":39,"start_character":0,"end_line":45,"end_character":33},"updated":"2017-05-30 20:01:47.000000000","message":"Does this assume certain fields (like \u0027total\u0027) are required and will therefore always be present?  I would expect we would want to return True if cur doesn\u0027t have \u0027total\u0027, but new does.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a2cce1e869a312055ff835fa81e786f7e005e39d","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        new_rec \u003d new[key]"},{"line_number":37,"context_line":"        for rec_key, cur_val in cur_rec.items():"},{"line_number":38,"context_line":"            if rec_key not in new_rec:"},{"line_number":39,"context_line":"                # Deliberately don\u0027t want to compare missing keys in the"},{"line_number":40,"context_line":"                # inventory record. For instance, we will be passing in fields"},{"line_number":41,"context_line":"                # like allocation_ratio in the current dict but the resource"},{"line_number":42,"context_line":"                # tracker may only pass in the total field. We want to return"},{"line_number":43,"context_line":"                # that inventory didn\u0027t change when the total field values are"},{"line_number":44,"context_line":"                # the same even if the allocation_ratio field is missing from"},{"line_number":45,"context_line":"                # the new record."},{"line_number":46,"context_line":"                continue"},{"line_number":47,"context_line":"            if new_rec[rec_key] !\u003d cur_val:"},{"line_number":48,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_94cceb6a","line":45,"range":{"start_line":39,"start_character":0,"end_line":45,"end_character":33},"in_reply_to":"df140735_0a810790","updated":"2017-06-01 14:36:53.000000000","message":"Right, so that\u0027s my concern.  If the old inventory didn\u0027t have \u0027total\u0027, but the new inventory does, I would think you would want to return True.  If things like \u0027total\u0027, \u0027min_unit\u0027, and \u0027max_unit\u0027 are guaranteed to be there, that should be mentioned in this comment.  If they\u0027re not guaranteed to be there, then the algorithm should account for the \"important\" fields differently.\n\nUnless my premise is wrong, and it\u0027s not considered an inventory change when I add total/min_unit/max_unit where it was previously missing.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        new_rec \u003d new[key]"},{"line_number":37,"context_line":"        for rec_key, cur_val in cur_rec.items():"},{"line_number":38,"context_line":"            if rec_key not in new_rec:"},{"line_number":39,"context_line":"                # Deliberately don\u0027t want to compare missing keys in the"},{"line_number":40,"context_line":"                # inventory record. For instance, we will be passing in fields"},{"line_number":41,"context_line":"                # like allocation_ratio in the current dict but the resource"},{"line_number":42,"context_line":"                # tracker may only pass in the total field. We want to return"},{"line_number":43,"context_line":"                # that inventory didn\u0027t change when the total field values are"},{"line_number":44,"context_line":"                # the same even if the allocation_ratio field is missing from"},{"line_number":45,"context_line":"                # the new record."},{"line_number":46,"context_line":"                continue"},{"line_number":47,"context_line":"            if new_rec[rec_key] !\u003d cur_val:"},{"line_number":48,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_0a810790","line":45,"range":{"start_line":39,"start_character":0,"end_line":45,"end_character":33},"in_reply_to":"df140735_6518bf1c","updated":"2017-06-01 00:34:02.000000000","message":"It means precisely what it says :) We only consider fields that are present in both dicts.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"961e0f3f93ce5a9b5dd5d3c3e2b46799c33104b7","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        new_rec \u003d new[key]"},{"line_number":37,"context_line":"        for rec_key, cur_val in cur_rec.items():"},{"line_number":38,"context_line":"            if rec_key not in new_rec:"},{"line_number":39,"context_line":"                # Deliberately don\u0027t want to compare missing keys in the"},{"line_number":40,"context_line":"                # inventory record. For instance, we will be passing in fields"},{"line_number":41,"context_line":"                # like allocation_ratio in the current dict but the resource"},{"line_number":42,"context_line":"                # tracker may only pass in the total field. We want to return"},{"line_number":43,"context_line":"                # that inventory didn\u0027t change when the total field values are"},{"line_number":44,"context_line":"                # the same even if the allocation_ratio field is missing from"},{"line_number":45,"context_line":"                # the new record."},{"line_number":46,"context_line":"                continue"},{"line_number":47,"context_line":"            if new_rec[rec_key] !\u003d cur_val:"},{"line_number":48,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_99075783","line":45,"range":{"start_line":39,"start_character":0,"end_line":45,"end_character":33},"in_reply_to":"df140735_94cceb6a","updated":"2017-06-02 15:49:23.000000000","message":"I have just the opposite question. In L115 you are overwriting self.inventory with the new value. What happens with those fields present in the old inventary but not in the new one?","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"99605c883c35e7e64a966a374262abb838a7abee","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        new_rec \u003d new[key]"},{"line_number":37,"context_line":"        for rec_key, cur_val in cur_rec.items():"},{"line_number":38,"context_line":"            if rec_key not in new_rec:"},{"line_number":39,"context_line":"                # Deliberately don\u0027t want to compare missing keys in the"},{"line_number":40,"context_line":"                # inventory record. For instance, we will be passing in fields"},{"line_number":41,"context_line":"                # like allocation_ratio in the current dict but the resource"},{"line_number":42,"context_line":"                # tracker may only pass in the total field. We want to return"},{"line_number":43,"context_line":"                # that inventory didn\u0027t change when the total field values are"},{"line_number":44,"context_line":"                # the same even if the allocation_ratio field is missing from"},{"line_number":45,"context_line":"                # the new record."},{"line_number":46,"context_line":"                continue"},{"line_number":47,"context_line":"            if new_rec[rec_key] !\u003d cur_val:"},{"line_number":48,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_3ab839e3","line":45,"range":{"start_line":39,"start_character":0,"end_line":45,"end_character":33},"in_reply_to":"df140735_99075783","updated":"2017-06-02 17:27:59.000000000","message":"This is all due to the messed up way we allow some things to be controlled by the resource tracker and other things to be controlled by the virt driver :( I\u0027ll try to pull this out and into a separate patch where we can reason about it.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        with self.tree.lock:"},{"line_number":69,"context_line":"            self.generation \u003d generation"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def find_by_uuid(self, uuid):"},{"line_number":72,"context_line":"        with self.tree.lock:"},{"line_number":73,"context_line":"            if self.uuid \u003d\u003d uuid:"},{"line_number":74,"context_line":"                return self"},{"line_number":75,"context_line":"            if uuid in self.children:"},{"line_number":76,"context_line":"                return self.children[uuid]"},{"line_number":77,"context_line":"        if self.children:"},{"line_number":78,"context_line":"            for child in self.children.values():"},{"line_number":79,"context_line":"                subchild \u003d child.find_by_uuid(uuid)"},{"line_number":80,"context_line":"                if subchild:"},{"line_number":81,"context_line":"                    return subchild"},{"line_number":82,"context_line":"        return None"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def find_by_name(self, name):"},{"line_number":85,"context_line":"        with self.tree.lock:"},{"line_number":86,"context_line":"            if self.name \u003d\u003d name:"},{"line_number":87,"context_line":"                return self"},{"line_number":88,"context_line":"            if name in self.children_name_map:"},{"line_number":89,"context_line":"                return self.children_name_map[name]"},{"line_number":90,"context_line":"        if self.children:"},{"line_number":91,"context_line":"            for child in self.children.values():"},{"line_number":92,"context_line":"                subchild \u003d child.find_by_name(name)"},{"line_number":93,"context_line":"                if subchild:"},{"line_number":94,"context_line":"                    return subchild"},{"line_number":95,"context_line":"        return None"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def add_child(self, provider):"},{"line_number":98,"context_line":"        with self.tree.lock:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_2551a737","line":95,"range":{"start_line":71,"start_character":0,"end_line":95,"end_character":19},"updated":"2017-05-30 20:01:47.000000000","message":"Couple of issues here.\n- Don\u0027t we need to hold the lock throughout, rather than releasing \u0026 reacquiring it to walk grandchildren?\n- These two methods are the same algo, just with different inputs.  Consolidate.\n- (Maybe) If breadth vs. depth doesn\u0027t matter, we don\u0027t need the middle chunk (if uuid in self.children).\n\n def _find_with_lock(self, key, value, child_dict_attr):\n     # Called with lock already held.\n     if getattr(self, key) \u003d\u003d value:\n         return self\n     for child in getattr(self, child_dict_attr).values():\n         found \u003d child._find_with_lock(key, value, child_dict_attr)\n         if found:\n             return found\n     return None\n\n def find_by_uuid(self, uuid):\n     with self.tree.lock:\n         return self._find_with_lock(\u0027uuid\u0027, uuid, \u0027children\u0027)\n\n def find_by_name(self, name):\n     with self.tree.lock:\n         return self._find_with_lock(\u0027name\u0027, name, \u0027children_name_map\u0027)","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        with self.tree.lock:"},{"line_number":69,"context_line":"            self.generation \u003d generation"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def find_by_uuid(self, uuid):"},{"line_number":72,"context_line":"        with self.tree.lock:"},{"line_number":73,"context_line":"            if self.uuid \u003d\u003d uuid:"},{"line_number":74,"context_line":"                return self"},{"line_number":75,"context_line":"            if uuid in self.children:"},{"line_number":76,"context_line":"                return self.children[uuid]"},{"line_number":77,"context_line":"        if self.children:"},{"line_number":78,"context_line":"            for child in self.children.values():"},{"line_number":79,"context_line":"                subchild \u003d child.find_by_uuid(uuid)"},{"line_number":80,"context_line":"                if subchild:"},{"line_number":81,"context_line":"                    return subchild"},{"line_number":82,"context_line":"        return None"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def find_by_name(self, name):"},{"line_number":85,"context_line":"        with self.tree.lock:"},{"line_number":86,"context_line":"            if self.name \u003d\u003d name:"},{"line_number":87,"context_line":"                return self"},{"line_number":88,"context_line":"            if name in self.children_name_map:"},{"line_number":89,"context_line":"                return self.children_name_map[name]"},{"line_number":90,"context_line":"        if self.children:"},{"line_number":91,"context_line":"            for child in self.children.values():"},{"line_number":92,"context_line":"                subchild \u003d child.find_by_name(name)"},{"line_number":93,"context_line":"                if subchild:"},{"line_number":94,"context_line":"                    return subchild"},{"line_number":95,"context_line":"        return None"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def add_child(self, provider):"},{"line_number":98,"context_line":"        with self.tree.lock:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_aa1d7b74","line":95,"range":{"start_line":71,"start_character":0,"end_line":95,"end_character":19},"in_reply_to":"df140735_2551a737","updated":"2017-06-01 00:34:02.000000000","message":"Good points above, Eric. I\u0027ll go with your suggested code. And yes, you\u0027re right about needing to hold the lock, otherwise I\u0027d need to do:\n\n for child in copy.deepcopy(self.children.values()):","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":94,"context_line":"                    return subchild"},{"line_number":95,"context_line":"        return None"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def add_child(self, provider):"},{"line_number":98,"context_line":"        with self.tree.lock:"},{"line_number":99,"context_line":"            self.children[provider.uuid] \u003d provider"},{"line_number":100,"context_line":"            self.children_name_map[provider.name] \u003d provider"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_202d1595","line":97,"updated":"2017-05-30 20:01:47.000000000","message":"Could have this guy set provider.parent_uuid \u003d self.uuid, save some processing in ProviderTree.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":99,"context_line":"            self.children[provider.uuid] \u003d provider"},{"line_number":100,"context_line":"            self.children_name_map[provider.name] \u003d provider"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def remove_child(self, provider):"},{"line_number":103,"context_line":"        with self.tree.lock:"},{"line_number":104,"context_line":"            if provider.uuid in self.children:"},{"line_number":105,"context_line":"                del self.children_name_map[provider.name]"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_c5242bc9","line":102,"updated":"2017-05-30 20:01:47.000000000","message":"Should this guy recurse down (find \u0026 delete `provider` no matter where it is in the hierarchy)?","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"99605c883c35e7e64a966a374262abb838a7abee","unresolved":false,"context_lines":[{"line_number":99,"context_line":"            self.children[provider.uuid] \u003d provider"},{"line_number":100,"context_line":"            self.children_name_map[provider.name] \u003d provider"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def remove_child(self, provider):"},{"line_number":103,"context_line":"        with self.tree.lock:"},{"line_number":104,"context_line":"            if provider.uuid in self.children:"},{"line_number":105,"context_line":"                del self.children_name_map[provider.name]"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_9a0625ce","line":102,"in_reply_to":"df140735_c5242bc9","updated":"2017-06-02 17:27:59.000000000","message":"The tree operation does that...","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        \"\"\"Safely removes the provider identified by the supplied name_or_uuid"},{"line_number":141,"context_line":"        parameter and all of its children from the tree."},{"line_number":142,"context_line":"        \"\"\""},{"line_number":143,"context_line":"        found \u003d self.find(name_or_uuid)"},{"line_number":144,"context_line":"        if not found:"},{"line_number":145,"context_line":"            raise ValueError(\"No such provider %s\" % name_or_uuid)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if found.parent_uuid:"},{"line_number":148,"context_line":"            parent \u003d self.find(found.parent_uuid)"},{"line_number":149,"context_line":"            parent.remove_child(found)"},{"line_number":150,"context_line":"        else:"},{"line_number":151,"context_line":"            with self.lock:"},{"line_number":152,"context_line":"                self.roots.remove(found)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    def new_root(self, name, uuid, generation):"},{"line_number":155,"context_line":"        \"\"\"Adds a new root provider to the tree.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_60478d4d","line":152,"range":{"start_line":143,"start_character":0,"end_line":152,"end_character":40},"updated":"2017-05-30 20:01:47.000000000","message":"Ayee, this whole thing should be done under self.lock.  Else you have a race with new_root and new_child.\n\nStarting to think _Provider should never hold self.tree.lock - do all locking from ProviderTree.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        \"\"\"Safely removes the provider identified by the supplied name_or_uuid"},{"line_number":141,"context_line":"        parameter and all of its children from the tree."},{"line_number":142,"context_line":"        \"\"\""},{"line_number":143,"context_line":"        found \u003d self.find(name_or_uuid)"},{"line_number":144,"context_line":"        if not found:"},{"line_number":145,"context_line":"            raise ValueError(\"No such provider %s\" % name_or_uuid)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        if found.parent_uuid:"},{"line_number":148,"context_line":"            parent \u003d self.find(found.parent_uuid)"},{"line_number":149,"context_line":"            parent.remove_child(found)"},{"line_number":150,"context_line":"        else:"},{"line_number":151,"context_line":"            with self.lock:"},{"line_number":152,"context_line":"                self.roots.remove(found)"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    def new_root(self, name, uuid, generation):"},{"line_number":155,"context_line":"        \"\"\"Adds a new root provider to the tree.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_b52ba080","line":152,"range":{"start_line":143,"start_character":0,"end_line":152,"end_character":40},"in_reply_to":"df140735_60478d4d","updated":"2017-06-01 00:34:02.000000000","message":"Yep, good points all around.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    def new_root(self, name, uuid, generation):"},{"line_number":155,"context_line":"        \"\"\"Adds a new root provider to the tree.\"\"\""},{"line_number":156,"context_line":"        if self.exists(uuid):"},{"line_number":157,"context_line":"            raise ValueError(\"Provider %s already exists as a root.\" % uuid)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        p \u003d _Provider(self, name, uuid, generation)"},{"line_number":160,"context_line":"        with self.lock:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_8063c1dc","line":157,"range":{"start_line":157,"start_character":29,"end_line":157,"end_character":75},"updated":"2017-05-30 20:01:47.000000000","message":"We still translate exception messages, right?\n\nAlso, the name of the conflicting provider would be nice to have here.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        \"\"\""},{"line_number":180,"context_line":"        return self.find(name_or_uuid) is not None"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def new_child(self, name, parent_uuid, uuid\u003dNone, generation\u003dNone):"},{"line_number":183,"context_line":"        \"\"\"Creates a new child provider with the given name and uuid under the"},{"line_number":184,"context_line":"        given parent."},{"line_number":185,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_80b8612a","line":182,"updated":"2017-05-30 20:01:47.000000000","message":"This could be consolidated with new_root.  If parent_uuid is None, self.roots.append; else self.find(parent_uuid).add_child.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        \"\"\""},{"line_number":208,"context_line":"        p \u003d self.find(name_or_uuid)"},{"line_number":209,"context_line":"        if not p:"},{"line_number":210,"context_line":"            raise ValueError(\"No such provider %s\" % name_or_uuid)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        return p.inventory_changed(inventory)"},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_4084895e","line":210,"range":{"start_line":210,"start_character":29,"end_line":210,"end_character":65},"updated":"2017-05-30 20:01:47.000000000","message":"translate?","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        \"\"\""},{"line_number":208,"context_line":"        p \u003d self.find(name_or_uuid)"},{"line_number":209,"context_line":"        if not p:"},{"line_number":210,"context_line":"            raise ValueError(\"No such provider %s\" % name_or_uuid)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        return p.inventory_changed(inventory)"},{"line_number":213,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_353790e6","line":210,"range":{"start_line":210,"start_character":29,"end_line":210,"end_character":65},"in_reply_to":"df140735_4084895e","updated":"2017-06-01 00:34:02.000000000","message":"Done","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"50504f6541f6d4d17fb0c9a848efe1f2ce2a63b7","unresolved":false,"context_lines":[{"line_number":81,"context_line":"            for child in self.children.values():"},{"line_number":82,"context_line":"                # We already searched for the child by UUID above, so here we"},{"line_number":83,"context_line":"                # just check for a child name match"},{"line_number":84,"context_line":"                if child.name \u003d\u003d search:"},{"line_number":85,"context_line":"                    return child"},{"line_number":86,"context_line":"                subchild \u003d child._find(search, search_key)"},{"line_number":87,"context_line":"                if subchild:"}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_82660529","line":84,"range":{"start_line":84,"start_character":0,"end_line":84,"end_character":40},"updated":"2017-05-31 19:42:06.000000000","message":"So you\u0027ll wind up comparing the name an extra time for any layer of the hierarchy that doesn\u0027t have what you\u0027re looking for.  That\u0027s probably not a big deal, but worth noting.","commit_id":"8fb21041320111dccdb4fa460710e860e9a87d4c"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"50504f6541f6d4d17fb0c9a848efe1f2ce2a63b7","unresolved":false,"context_lines":[{"line_number":148,"context_line":"    def new_root(self, name, uuid, generation):"},{"line_number":149,"context_line":"        \"\"\"Adds a new root provider to the tree.\"\"\""},{"line_number":150,"context_line":"        with self.lock:"},{"line_number":151,"context_line":"            if self._find_with_lock(uuid) is not None:"},{"line_number":152,"context_line":"                raise ValueError("},{"line_number":153,"context_line":"                    _(\"Provider %s already exists as a root.\") % uuid"},{"line_number":154,"context_line":"                )"}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_bd45ac45","line":151,"range":{"start_line":151,"start_character":15,"end_line":151,"end_character":54},"updated":"2017-05-31 19:42:06.000000000","message":"Are names guaranteed to be unique and uniquely bound to their respective UUID?  I\u0027m not seeing any enforcement/checking for that in here.","commit_id":"8fb21041320111dccdb4fa460710e860e9a87d4c"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"50504f6541f6d4d17fb0c9a848efe1f2ce2a63b7","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        with self.lock:"},{"line_number":174,"context_line":"            return self._find_with_lock(name_or_uuid)"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def exists(self, name_or_uuid):"},{"line_number":177,"context_line":"        \"\"\"Given either a name or a UUID, return True if the tree contains the"},{"line_number":178,"context_line":"        child provider, False otherwise."},{"line_number":179,"context_line":"        \"\"\""},{"line_number":180,"context_line":"        with self.lock:"},{"line_number":181,"context_line":"            found \u003d self._find_with_lock(name_or_uuid)"},{"line_number":182,"context_line":"        return found is not None"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def new_child(self, name, parent_uuid, uuid\u003dNone, generation\u003dNone):"},{"line_number":185,"context_line":"        \"\"\"Creates a new child provider with the given name and uuid under the"}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_22605901","line":182,"range":{"start_line":176,"start_character":0,"end_line":182,"end_character":32},"updated":"2017-05-31 19:42:06.000000000","message":"This seems kinda extraneous - unless you\u0027re going to add name and/or UUID caches.  Don\u0027t know how big we\u0027re expecting these things to get, or how crucial performance is, but if you maintained a set of all the names/UUIDs in the tree, you could make this guy O(1), and save similarly on the uniqueness checks in new_child/new_root.","commit_id":"8fb21041320111dccdb4fa460710e860e9a87d4c"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"50504f6541f6d4d17fb0c9a848efe1f2ce2a63b7","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        :raises ValueError if parent_uuid points to a non-existing provider."},{"line_number":191,"context_line":"        \"\"\""},{"line_number":192,"context_line":"        with self.lock:"},{"line_number":193,"context_line":"            parent \u003d self._find_with_lock(parent_uuid)"},{"line_number":194,"context_line":"            if not parent:"},{"line_number":195,"context_line":"                raise ValueError(_(\"No such parent %s\") % parent_uuid)"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"df140735_fd3684e7","line":193,"range":{"start_line":193,"start_character":12,"end_line":193,"end_character":54},"updated":"2017-05-31 19:42:06.000000000","message":"Ditto (name uniqueness)","commit_id":"8fb21041320111dccdb4fa460710e860e9a87d4c"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9700cc02481fdab57b43efef107d4f02082bbd95","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        # dict of inventory records, keyed by resource class"},{"line_number":70,"context_line":"        self.inventory \u003d {}"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def set_generation(self, generation):"},{"line_number":73,"context_line":"        self.generation \u003d generation"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _find(self, search, search_key):"},{"line_number":76,"context_line":"        if getattr(self, search_key) \u003d\u003d search:"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_4e38d3ec","line":73,"range":{"start_line":72,"start_character":0,"end_line":73,"end_character":36},"updated":"2017-06-02 16:00:39.000000000","message":"Is there a reason for this method? \u0027generation\u0027 is a public attribute of the class, so anything that can call this can simply set it directly.","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d18fc5c33e10ceabe9df42d8cf89f97fca6e442","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        # dict of inventory records, keyed by resource class"},{"line_number":70,"context_line":"        self.inventory \u003d {}"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def set_generation(self, generation):"},{"line_number":73,"context_line":"        self.generation \u003d generation"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _find(self, search, search_key):"},{"line_number":76,"context_line":"        if getattr(self, search_key) \u003d\u003d search:"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_bf7afbb6","line":73,"range":{"start_line":72,"start_character":0,"end_line":73,"end_character":36},"in_reply_to":"df140735_4e38d3ec","updated":"2017-06-02 17:07:27.000000000","message":"Mostly leftover from previous thoughts I think. I actually want this method gone and no callers of _Provider methods at all other than ProviderTree because it holds the lock on the tree. So I\u0027ll put this method on the provider tree object.","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9700cc02481fdab57b43efef107d4f02082bbd95","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.generation \u003d generation"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _find(self, search, search_key):"},{"line_number":76,"context_line":"        if getattr(self, search_key) \u003d\u003d search:"},{"line_number":77,"context_line":"            return self"},{"line_number":78,"context_line":"        if search in self.children:"},{"line_number":79,"context_line":"            return self.children[search]"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_d904af1a","line":76,"updated":"2017-06-02 16:00:39.000000000","message":"This will raise an AttributeError if the _Provider object doesn\u0027t have that attribute. Specifically, if we are searching by uuid, and a provider doesn\u0027t have one, this will crash.","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d18fc5c33e10ceabe9df42d8cf89f97fca6e442","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.generation \u003d generation"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _find(self, search, search_key):"},{"line_number":76,"context_line":"        if getattr(self, search_key) \u003d\u003d search:"},{"line_number":77,"context_line":"            return self"},{"line_number":78,"context_line":"        if search in self.children:"},{"line_number":79,"context_line":"            return self.children[search]"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_9f64d723","line":76,"in_reply_to":"df140735_d904af1a","updated":"2017-06-02 17:07:27.000000000","message":"in what cases will a _Provider not have a uuid or name attribute? see the above constructor.","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9700cc02481fdab57b43efef107d4f02082bbd95","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        self.children[provider.uuid] \u003d provider"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def remove_child(self, provider):"},{"line_number":101,"context_line":"        if provider.uuid in self.children:"},{"line_number":102,"context_line":"            del self.children[provider.uuid]"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def inventory_changed(self, inventory):"},{"line_number":105,"context_line":"        \"\"\"Update the stored inventory for the provider and return whether the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_d9602fd2","line":102,"range":{"start_line":101,"start_character":8,"end_line":102,"end_character":43},"updated":"2017-06-02 16:00:39.000000000","message":"Since you don\u0027t care if the child doesn\u0027t exist, this could be simplified to:\n\n  self.children.pop(provider.uuid, None)","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9700cc02481fdab57b43efef107d4f02082bbd95","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        \"\"\"Update the stored inventory for the provider and return whether the"},{"line_number":106,"context_line":"        inventory has changed."},{"line_number":107,"context_line":"        \"\"\""},{"line_number":108,"context_line":"        return _inventory_has_changed(self.inventory, inventory)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def update_inventory(self, inventory):"},{"line_number":111,"context_line":"        \"\"\"Update the stored inventory for the provider and return whether the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_199907c3","line":108,"range":{"start_line":108,"start_character":15,"end_line":108,"end_character":37},"updated":"2017-06-02 16:00:39.000000000","message":"Curious about the slight name difference. The other related methods are just \u0027inventory_changed\u0027, but this adds \u0027has\u0027 to the name. Personally, I prefer \u0027has_inventory_changed\u0027 for all, since the method is simply answering that question.","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d18fc5c33e10ceabe9df42d8cf89f97fca6e442","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        \"\"\"Update the stored inventory for the provider and return whether the"},{"line_number":106,"context_line":"        inventory has changed."},{"line_number":107,"context_line":"        \"\"\""},{"line_number":108,"context_line":"        return _inventory_has_changed(self.inventory, inventory)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def update_inventory(self, inventory):"},{"line_number":111,"context_line":"        \"\"\"Update the stored inventory for the provider and return whether the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_9f4df79b","line":108,"range":{"start_line":108,"start_character":15,"end_line":108,"end_character":37},"in_reply_to":"df140735_199907c3","updated":"2017-06-02 17:07:27.000000000","message":"Done","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9700cc02481fdab57b43efef107d4f02082bbd95","unresolved":false,"context_lines":[{"line_number":120,"context_line":"class ProviderTree(object):"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    def __init__(self, cns\u003dNone):"},{"line_number":123,"context_line":"        \"\"\"Create a provider tree from an `objects.ComputeNodeList` object.\"\"\""},{"line_number":124,"context_line":"        self.lock \u003d lockutils.internal_lock(_LOCK_NAME)"},{"line_number":125,"context_line":"        self.roots \u003d []"},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_d9514fdb","line":123,"range":{"start_line":123,"start_character":11,"end_line":123,"end_character":74},"updated":"2017-06-02 16:00:39.000000000","message":"If the design here is to be limited to compute nodes, perhaps the class should be ComputeNodeProviderTree, with a more generic superclass for non-ComputeNode providers?","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d18fc5c33e10ceabe9df42d8cf89f97fca6e442","unresolved":false,"context_lines":[{"line_number":120,"context_line":"class ProviderTree(object):"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    def __init__(self, cns\u003dNone):"},{"line_number":123,"context_line":"        \"\"\"Create a provider tree from an `objects.ComputeNodeList` object.\"\"\""},{"line_number":124,"context_line":"        self.lock \u003d lockutils.internal_lock(_LOCK_NAME)"},{"line_number":125,"context_line":"        self.roots \u003d []"},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_df56cf2a","line":123,"range":{"start_line":123,"start_character":11,"end_line":123,"end_character":74},"in_reply_to":"df140735_d9514fdb","updated":"2017-06-02 17:07:27.000000000","message":"No, the design is specifically *not* limited to compute nodes. It\u0027s just that the initial roots of the tree will be currently a list of compute nodes (coming from resource_tracker\u0027s ComputeNodeList.get_all_by_host() call)","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9700cc02481fdab57b43efef107d4f02082bbd95","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        \"\"\""},{"line_number":180,"context_line":"        with self.lock:"},{"line_number":181,"context_line":"            found \u003d self._find_with_lock(name_or_uuid)"},{"line_number":182,"context_line":"        return found is not None"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def new_child(self, name, parent_uuid, uuid\u003dNone, generation\u003dNone):"},{"line_number":185,"context_line":"        \"\"\"Creates a new child provider with the given name and uuid under the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_b9157be7","line":182,"range":{"start_line":182,"start_character":8,"end_line":182,"end_character":32},"updated":"2017-06-02 16:00:39.000000000","message":"return bool(found)","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d18fc5c33e10ceabe9df42d8cf89f97fca6e442","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        \"\"\""},{"line_number":180,"context_line":"        with self.lock:"},{"line_number":181,"context_line":"            found \u003d self._find_with_lock(name_or_uuid)"},{"line_number":182,"context_line":"        return found is not None"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def new_child(self, name, parent_uuid, uuid\u003dNone, generation\u003dNone):"},{"line_number":185,"context_line":"        \"\"\"Creates a new child provider with the given name and uuid under the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_7f37c300","line":182,"range":{"start_line":182,"start_character":8,"end_line":182,"end_character":32},"in_reply_to":"df140735_b9157be7","updated":"2017-06-02 17:07:27.000000000","message":"prefer what\u0027s there frankly.","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9700cc02481fdab57b43efef107d4f02082bbd95","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def inventory_changed(self, name_or_uuid, inventory):"},{"line_number":202,"context_line":"        \"\"\"Returns True if the supplied inventory is different for the provider"},{"line_number":203,"context_line":"        with the supplied name or UUID."},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        :raises: ValueError if a provider with uuid was not found in the tree."},{"line_number":206,"context_line":"        :param name_or_uuid: Either name or UUID of the resource provider to"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_79a7e3be","line":203,"updated":"2017-06-02 16:00:39.000000000","message":"Since this is the public method that people will use, I would add some wording similar to the comment _inventory_has_changed() about only comparing keys that both supplied inv and provider inv have in common.","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1d18fc5c33e10ceabe9df42d8cf89f97fca6e442","unresolved":false,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def inventory_changed(self, name_or_uuid, inventory):"},{"line_number":202,"context_line":"        \"\"\"Returns True if the supplied inventory is different for the provider"},{"line_number":203,"context_line":"        with the supplied name or UUID."},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        :raises: ValueError if a provider with uuid was not found in the tree."},{"line_number":206,"context_line":"        :param name_or_uuid: Either name or UUID of the resource provider to"}],"source_content_type":"text/x-python","patch_set":11,"id":"df140735_5f387fec","line":203,"in_reply_to":"df140735_79a7e3be","updated":"2017-06-02 17:07:27.000000000","message":"Done","commit_id":"b0c221b9bf29238b014dd71b6c67e752f889f4b5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1c546bdce7f404e5fbfa00bc20bee68bd0fcbdfa","unresolved":false,"context_lines":[{"line_number":16,"context_line":"object is constructed and used by the scheduler report client to track state"},{"line_number":17,"context_line":"changes for resources on the hypervisor or baremetal node. As such, there are"},{"line_number":18,"context_line":"no remoteable methods nor is there any interaction with the nova.db modules."},{"line_number":19,"context_line":"\"\"\""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from oslo_concurrency import lockutils"},{"line_number":22,"context_line":"from oslo_log import log as logging"}],"source_content_type":"text/x-python","patch_set":13,"id":"df140735_f901573b","line":19,"updated":"2017-06-06 17:05:28.000000000","message":"While I totally get the intent of that helper module and why you don\u0027t want to have those methods be part of the ResourceProvider object, I still wonder whether we should have such semantics into a totally separate module which would be in the compute namespace.\nIf we go that way, I\u0027d consider making it enough abstract (it\u0027s just a library for walking within a tree and updating its leafs, right?) and put it in some other namespace (like nova.objects even if it\u0027s not inheriting from o.vo).\n\nFWIW, I\u0027m also trying to recollect in my mind all the existing Python libraries that do such things.","commit_id":"d28c8dd2b9fac676f5e8aca6551bfe8b4da78a89"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"eaf25bcc48523db5cb5a8e552434d77517824b4a","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            self.roots.append(p)"},{"line_number":115,"context_line":"            return p"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def _find_with_lock(self, name_or_uuid):"},{"line_number":118,"context_line":"        if uuidutils.is_uuid_like(name_or_uuid):"},{"line_number":119,"context_line":"            getter \u003d \u0027find_by_uuid\u0027"},{"line_number":120,"context_line":"        else:"},{"line_number":121,"context_line":"            getter \u003d \u0027find_by_name\u0027"},{"line_number":122,"context_line":"        for root in self.roots:"},{"line_number":123,"context_line":"            fn \u003d getattr(root, getter)"},{"line_number":124,"context_line":"            found \u003d fn(name_or_uuid)"},{"line_number":125,"context_line":"            if found:"},{"line_number":126,"context_line":"                return found"},{"line_number":127,"context_line":"        return None"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def find(self, name_or_uuid):"},{"line_number":130,"context_line":"        with self.lock:"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f231b9d_6ba1b7a9","line":127,"range":{"start_line":117,"start_character":0,"end_line":127,"end_character":19},"updated":"2017-06-12 14:18:58.000000000","message":"nit: this should probably go into the \u0027_Provider\u0027 class as \u0027find\u0027 or similar?","commit_id":"d28c8dd2b9fac676f5e8aca6551bfe8b4da78a89"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08659cf4a8d023eefec3d4e0feee73547c1075e0","unresolved":false,"context_lines":[{"line_number":38,"context_line":"            uuid \u003d uuidutils.generate_uuid()"},{"line_number":39,"context_line":"        self.uuid \u003d uuid"},{"line_number":40,"context_line":"        self.name \u003d name"},{"line_number":41,"context_line":"        self.generation \u003d generation"},{"line_number":42,"context_line":"        self.parent_uuid \u003d parent_uuid"},{"line_number":43,"context_line":"        # Contains a dict, keyed by uuid of child resource providers having"},{"line_number":44,"context_line":"        # this provider as a parent"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f1d235d_f1903f51","line":41,"updated":"2017-06-29 10:17:13.000000000","message":"generation is not really needed, right? this tree just needs indexes (name and/or uuids) for walking it and what the leaf is is really an ubiquitous object, unless you find generation needed for locking ?","commit_id":"4bcb520550617987c7e34f7bd959ffd78c28c756"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08659cf4a8d023eefec3d4e0feee73547c1075e0","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                # just check for a child name match"},{"line_number":56,"context_line":"                if child.name \u003d\u003d search:"},{"line_number":57,"context_line":"                    return child"},{"line_number":58,"context_line":"                subchild \u003d child._find(search, search_key)"},{"line_number":59,"context_line":"                if subchild:"},{"line_number":60,"context_line":"                    return subchild"},{"line_number":61,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f1d235d_7b665054","line":58,"updated":"2017-06-29 10:17:13.000000000","message":"note that because Python, if we have more than 1000 recursions, it will provide an exception, but I don\u0027t think we will have ResourceProviders having more than grand-children by a 1000 depth :-)","commit_id":"4bcb520550617987c7e34f7bd959ffd78c28c756"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08659cf4a8d023eefec3d4e0feee73547c1075e0","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            del self.children[provider.uuid]"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"class ProviderTree(object):"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def __init__(self, cns\u003dNone):"},{"line_number":80,"context_line":"        \"\"\"Create a provider tree from an `objects.ComputeNodeList` object.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"3f1d235d_db47a4bc","line":77,"updated":"2017-06-29 10:17:13.000000000","message":"honestly, we could create some abstract object for walking within a tree and just use it for our ResourceProviders, but that\u0027s possibly something we could optimize it later.","commit_id":"4bcb520550617987c7e34f7bd959ffd78c28c756"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08659cf4a8d023eefec3d4e0feee73547c1075e0","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        if cns:"},{"line_number":85,"context_line":"            for cn in cns:"},{"line_number":86,"context_line":"                # By definition, all compute nodes are root providers..."},{"line_number":87,"context_line":"                p \u003d _Provider(cn.hypervisor_hostname, cn.uuid)"},{"line_number":88,"context_line":"                self.roots.append(p)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def remove(self, name_or_uuid):"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f1d235d_51e9337e","line":87,"range":{"start_line":87,"start_character":20,"end_line":87,"end_character":62},"updated":"2017-06-29 10:17:13.000000000","message":"see, you\u0027re even not setting the generation","commit_id":"4bcb520550617987c7e34f7bd959ffd78c28c756"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"70874b9c933c0dcc7e1cb6c0c0a6d7436ac65791","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        if cns:"},{"line_number":85,"context_line":"            for cn in cns:"},{"line_number":86,"context_line":"                # By definition, all compute nodes are root providers..."},{"line_number":87,"context_line":"                p \u003d _Provider(cn.hypervisor_hostname, cn.uuid)"},{"line_number":88,"context_line":"                self.roots.append(p)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def remove(self, name_or_uuid):"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f1d235d_601d1bbc","line":87,"range":{"start_line":87,"start_character":20,"end_line":87,"end_character":62},"in_reply_to":"3f1d235d_51e9337e","updated":"2017-07-03 16:08:00.000000000","message":"The generation gets set in a later patch:\n\nhttps://review.openstack.org/#/c/415921/15/nova/compute/provider_tree.py","commit_id":"4bcb520550617987c7e34f7bd959ffd78c28c756"}],"nova/tests/unit/compute/test_provider_tree.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":25,"context_line":"            hypervisor_hostname\u003d\u0027compute-node-1\u0027,"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        self.compute_nodes \u003d objects.ComputeNodeList("},{"line_number":28,"context_line":"            objects\u003d[self.compute_node],"},{"line_number":29,"context_line":"        )"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def test_tree_ops(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_20ee3522","line":28,"range":{"start_line":28,"start_character":21,"end_line":28,"end_character":38},"updated":"2017-05-30 20:01:47.000000000","message":"Suggest having more than one node here; else you\u0027re missing exercising a bunch of the logic.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":25,"context_line":"            hypervisor_hostname\u003d\u0027compute-node-1\u0027,"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        self.compute_nodes \u003d objects.ComputeNodeList("},{"line_number":28,"context_line":"            objects\u003d[self.compute_node],"},{"line_number":29,"context_line":"        )"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def test_tree_ops(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_95dd3c44","line":28,"range":{"start_line":28,"start_character":21,"end_line":28,"end_character":38},"in_reply_to":"df140735_20ee3522","updated":"2017-06-01 00:34:02.000000000","message":"Actually, I\u0027m not missing a whole lot of logic (other than running through the loop of compute nodes in the constructor...)\n\nYou will note that on line 77 below I\u0027m adding a new root...\n\nIn any case, I\u0027ll do what you\u0027re suggesting just to be sure.","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a313d57ccb519374e7bbfd53ecc15ef558f71293","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        numa_cell0 \u003d pt.new_child(\u0027numa_cell0\u0027, cn.uuid)"},{"line_number":50,"context_line":"        numa_cell1 \u003d pt.new_child(\u0027numa_cell1\u0027, cn.uuid)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self.assertEqual(numa_cell0, pt.find(\u0027numa_cell0\u0027))"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        self.assertTrue(pt.exists(numa_cell0.uuid))"},{"line_number":55,"context_line":"        self.assertTrue(pt.exists(\u0027numa_cell0\u0027))"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_80e62144","line":52,"range":{"start_line":52,"start_character":0,"end_line":52,"end_character":59},"updated":"2017-05-30 20:01:47.000000000","message":"And by uuid","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"495bc32e7954e06145e6bac35ed54ad411ce5895","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        numa_cell0 \u003d pt.new_child(\u0027numa_cell0\u0027, cn.uuid)"},{"line_number":50,"context_line":"        numa_cell1 \u003d pt.new_child(\u0027numa_cell1\u0027, cn.uuid)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self.assertEqual(numa_cell0, pt.find(\u0027numa_cell0\u0027))"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        self.assertTrue(pt.exists(numa_cell0.uuid))"},{"line_number":55,"context_line":"        self.assertTrue(pt.exists(\u0027numa_cell0\u0027))"}],"source_content_type":"text/x-python","patch_set":9,"id":"df140735_f516b8f1","line":52,"range":{"start_line":52,"start_character":0,"end_line":52,"end_character":59},"in_reply_to":"df140735_80e62144","updated":"2017-06-01 00:34:02.000000000","message":"Done","commit_id":"ca7301eec92ea232adb862fa462cbe26bd827b3d"}]}