)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f5bdd7d413fd952d34bda3a27ce7033b1ba0f77c","unresolved":false,"context_lines":[{"line_number":9,"context_line":"ProviderTree.populate_from_iterable accepts an iterable of provider"},{"line_number":10,"context_line":"dicts (like we get back from the placement API) and (re)populates the"},{"line_number":11,"context_line":"contents of the ProviderTree from them.  Existing entries (matched by"},{"line_number":12,"context_line":"UUID) are replaced; otherwise nothing is ever removed."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ifdcc8a713c3732c762a17c60cc4b2424078f5c23"},{"line_number":15,"context_line":"blueprint: nested-resource-providers"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"df87a7cf_6cfad475","line":12,"updated":"2017-12-06 18:46:28.000000000","message":"Why? What do we want to do with this? What happens if this functionality is not present? What happens when it is?","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"}],"nova/compute/provider_tree.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c7ccdfb8fb7857ad5bddfee0a2cccc66f0ca6cce","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        # We need this to validate the tree structure (no orphans)."},{"line_number":165,"context_line":"        all_uuids \u003d self.uuid_set()"},{"line_number":166,"context_line":"        for prov_dict in provider_dicts:"},{"line_number":167,"context_line":"            by_parent[prov_dict[\u0027parent_provider_uuid\u0027]].append(prov_dict)"},{"line_number":168,"context_line":"            all_uuids.add(prov_dict[\u0027uuid\u0027])"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        # Before we go under lock, sanity check to make sure all parents exist."}],"source_content_type":"text/x-python","patch_set":5,"id":"1f485f77_1cfc37b3","line":167,"range":{"start_line":167,"start_character":22,"end_line":167,"end_character":55},"updated":"2017-11-18 02:14:28.000000000","message":"It would seem as though placement does not give you\n\n \u0027parent_provider_uuid\u0027: None\n\nfor roots.  This needs to be .get().","commit_id":"829c6348c56a71617c115bd038d0323d6312cfc6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        # set(by_parent) is all the parents we need, including None for roots."},{"line_number":183,"context_line":"        # all_uuids is all the providers either already in the tree or in the"},{"line_number":184,"context_line":"        # input."},{"line_number":185,"context_line":"        # If the set difference is empty (other than None), it means the input"},{"line_number":186,"context_line":"        # contained providers with parents we don\u0027t have."},{"line_number":187,"context_line":"        missing_parents \u003d (set(by_parent) - all_uuids) - set([None])"},{"line_number":188,"context_line":"        if missing_parents:"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_7f2902e4","line":185,"range":{"start_line":185,"start_character":34,"end_line":185,"end_character":40},"updated":"2017-12-07 16:35:48.000000000","message":"not empty ?","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"20ddf2f5ed9d317470eb11153332bca70c49cb85","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        # set(by_parent) is all the parents we need, including None for roots."},{"line_number":183,"context_line":"        # all_uuids is all the providers either already in the tree or in the"},{"line_number":184,"context_line":"        # input."},{"line_number":185,"context_line":"        # If the set difference is empty (other than None), it means the input"},{"line_number":186,"context_line":"        # contained providers with parents we don\u0027t have."},{"line_number":187,"context_line":"        missing_parents \u003d (set(by_parent) - all_uuids) - set([None])"},{"line_number":188,"context_line":"        if missing_parents:"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_4bc9d623","line":185,"range":{"start_line":185,"start_character":34,"end_line":185,"end_character":40},"in_reply_to":"df87a7cf_7f2902e4","updated":"2017-12-07 21:16:56.000000000","message":"Yup, thanks.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                do_add \u003d parent.add_child"},{"line_number":198,"context_line":"            for pdict in pdicts:"},{"line_number":199,"context_line":"                try:"},{"line_number":200,"context_line":"                    self._remove_with_lock(pdict[\u0027uuid\u0027])"},{"line_number":201,"context_line":"                except ValueError:"},{"line_number":202,"context_line":"                    # Wasn\u0027t there in the first place - fine."},{"line_number":203,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_888da9d4","line":200,"updated":"2017-12-07 16:35:48.000000000","message":"When we remove this Provider then we lose the children dict of that Provider (_Provider.children). Then when we add this Provider back to the tree at L204 we create a new Provider with empty childer dict. Don\u0027t we loose children this way? Or we assume that if the input contains an RP then the input will also contain every child of the RP as well?","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"20ddf2f5ed9d317470eb11153332bca70c49cb85","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                do_add \u003d parent.add_child"},{"line_number":198,"context_line":"            for pdict in pdicts:"},{"line_number":199,"context_line":"                try:"},{"line_number":200,"context_line":"                    self._remove_with_lock(pdict[\u0027uuid\u0027])"},{"line_number":201,"context_line":"                except ValueError:"},{"line_number":202,"context_line":"                    # Wasn\u0027t there in the first place - fine."},{"line_number":203,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_eb0c6a42","line":200,"in_reply_to":"df87a7cf_888da9d4","updated":"2017-12-07 21:16:56.000000000","message":"The latter.  We do this on purpose, to account for the possibility of children changing (being added/removed).  I added words in the method docstring.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Ready to do the work."},{"line_number":209,"context_line":"        # Keep track of all the providers in the tree"},{"line_number":210,"context_line":"        provs_in_tree \u003d self.get_provider_uuids()"},{"line_number":211,"context_line":"        with self.lock:"},{"line_number":212,"context_line":"            # Use by_parent to keep track of what\u0027s still left to add"},{"line_number":213,"context_line":"            while by_parent:"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_dfa28dfc","line":210,"range":{"start_line":210,"start_character":8,"end_line":210,"end_character":21},"updated":"2017-12-07 16:35:48.000000000","message":"Isn\u0027t it the same collection as all_uuids that has already been gathered at L176?","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"20ddf2f5ed9d317470eb11153332bca70c49cb85","unresolved":false,"context_lines":[{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        # Ready to do the work."},{"line_number":209,"context_line":"        # Keep track of all the providers in the tree"},{"line_number":210,"context_line":"        provs_in_tree \u003d self.get_provider_uuids()"},{"line_number":211,"context_line":"        with self.lock:"},{"line_number":212,"context_line":"            # Use by_parent to keep track of what\u0027s still left to add"},{"line_number":213,"context_line":"            while by_parent:"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_5ef0b6fe","line":210,"range":{"start_line":210,"start_character":8,"end_line":210,"end_character":21},"in_reply_to":"df87a7cf_dfa28dfc","updated":"2017-12-07 21:16:56.000000000","message":"Initially, yes, but the one on L176 then gets filled out with UUIDs from the provider_dicts param.  all_uuids (L176) is only used to make sure we don\u0027t receive any orphans, which we do up front so we can control the error condition better.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        # Ready to do the work."},{"line_number":209,"context_line":"        # Keep track of all the providers in the tree"},{"line_number":210,"context_line":"        provs_in_tree \u003d self.get_provider_uuids()"},{"line_number":211,"context_line":"        with self.lock:"},{"line_number":212,"context_line":"            # Use by_parent to keep track of what\u0027s still left to add"},{"line_number":213,"context_line":"            while by_parent:"},{"line_number":214,"context_line":"                # Find roots, or an entry whose parent is already registered."}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_28ba5dc0","line":211,"updated":"2017-12-07 16:35:48.000000000","message":"I think the below algorithm needs some documentation to help the reader. My understanding is the following.\nAn RP can only be added to the tree if it\u0027s whole ancestry is already added. But the list of RPs provided in provider_dicts are not ordered in a way that a newly added parent always comes before it descendants. Therefore we have to always find those RPs in the input whose parent already in the tree and add those first.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"20ddf2f5ed9d317470eb11153332bca70c49cb85","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        # Ready to do the work."},{"line_number":209,"context_line":"        # Keep track of all the providers in the tree"},{"line_number":210,"context_line":"        provs_in_tree \u003d self.get_provider_uuids()"},{"line_number":211,"context_line":"        with self.lock:"},{"line_number":212,"context_line":"            # Use by_parent to keep track of what\u0027s still left to add"},{"line_number":213,"context_line":"            while by_parent:"},{"line_number":214,"context_line":"                # Find roots, or an entry whose parent is already registered."}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_7e001aac","line":211,"in_reply_to":"df87a7cf_28ba5dc0","updated":"2017-12-07 21:16:56.000000000","message":"Yes, good assessment.  I\u0027ll incorporate it into code comments.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                # Now those guys are in the tree and can be parents"},{"line_number":230,"context_line":"                provs_in_tree |\u003d set(prov[\u0027uuid\u0027] for prov in provs_to_add)"},{"line_number":231,"context_line":"                # And we\u0027re done with those; cross them off the to-do list."},{"line_number":232,"context_line":"                del by_parent[parent_uuid]"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _remove_with_lock(self, name_or_uuid):"},{"line_number":235,"context_line":"        found \u003d self._find_with_lock(name_or_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_8854c922","line":232,"updated":"2017-12-07 16:35:48.000000000","message":"Just not to myself: OK, it is not an infinite loop as we either throw a ValueError or remove an item from the dict we are looping on.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"20ddf2f5ed9d317470eb11153332bca70c49cb85","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                # Now those guys are in the tree and can be parents"},{"line_number":230,"context_line":"                provs_in_tree |\u003d set(prov[\u0027uuid\u0027] for prov in provs_to_add)"},{"line_number":231,"context_line":"                # And we\u0027re done with those; cross them off the to-do list."},{"line_number":232,"context_line":"                del by_parent[parent_uuid]"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _remove_with_lock(self, name_or_uuid):"},{"line_number":235,"context_line":"        found \u003d self._find_with_lock(name_or_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_fea8aa8c","line":232,"in_reply_to":"df87a7cf_8854c922","updated":"2017-12-07 21:16:56.000000000","message":"Correct.  I enhanced your paragraph above to include a note on that.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3ad5dadb001fc64497c0c9e1fa3546779216bce9","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            while by_parent:"},{"line_number":281,"context_line":"                # Find roots, or an entry whose parent is already registered."},{"line_number":282,"context_line":"                for parent_uuid, provs_to_add in by_parent.items():"},{"line_number":283,"context_line":"                    if parent_uuid is None or parent_uuid in provs_in_tree:"},{"line_number":284,"context_line":"                        break"},{"line_number":285,"context_line":"                else:"},{"line_number":286,"context_line":"                    # This should never happen - we already ensured all parents"}],"source_content_type":"text/x-python","patch_set":17,"id":"bf8cb3f7_930f63e0","line":283,"range":{"start_line":283,"start_character":20,"end_line":283,"end_character":75},"updated":"2017-12-25 12:54:59.000000000","message":"This risks the case that the child comes first and the parent comes later if the parent is already in the pt. And if that happens, the parent will kill the child.\n\nI quickly cooked a testcase patch for this.\nhttps://review.openstack.org/#/c/530035/","commit_id":"e42c49340961e272cbbd6aee0f937d016f0d1350"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e06a6fdd01f93e157c49ec736ab16822655090f6","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            while by_parent:"},{"line_number":281,"context_line":"                # Find roots, or an entry whose parent is already registered."},{"line_number":282,"context_line":"                for parent_uuid, provs_to_add in by_parent.items():"},{"line_number":283,"context_line":"                    if parent_uuid is None or parent_uuid in provs_in_tree:"},{"line_number":284,"context_line":"                        break"},{"line_number":285,"context_line":"                else:"},{"line_number":286,"context_line":"                    # This should never happen - we already ensured all parents"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_9cf3e273","line":283,"range":{"start_line":283,"start_character":20,"end_line":283,"end_character":75},"in_reply_to":"9f91af0f_331312b8","updated":"2018-01-04 23:12:30.000000000","message":"Well, that resulted in a nontrivial rework of this algorithm.  But (in addition to being correct) I think the result is actually cleaner and more readable.\n\nI\u0027ve incorporated your test case from https://review.openstack.org/#/c/530035/ and added you as co-author.\n\nThanks again for this excellent find.","commit_id":"e42c49340961e272cbbd6aee0f937d016f0d1350"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a761e28a3375eecaf3a7e3ab61912347d0db0885","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            while by_parent:"},{"line_number":281,"context_line":"                # Find roots, or an entry whose parent is already registered."},{"line_number":282,"context_line":"                for parent_uuid, provs_to_add in by_parent.items():"},{"line_number":283,"context_line":"                    if parent_uuid is None or parent_uuid in provs_in_tree:"},{"line_number":284,"context_line":"                        break"},{"line_number":285,"context_line":"                else:"},{"line_number":286,"context_line":"                    # This should never happen - we already ensured all parents"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f91af0f_331312b8","line":283,"range":{"start_line":283,"start_character":20,"end_line":283,"end_character":75},"in_reply_to":"bf8cb3f7_930f63e0","updated":"2018-01-04 16:24:25.000000000","message":"Thanks Tetsuro-san, this is an excellent point.  IIUC, it can\u0027t happen unless we call populate_from_iterable() on a nonempty tree.  I\u0027ll look into it.","commit_id":"e42c49340961e272cbbd6aee0f937d016f0d1350"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"f1282e4d87631ecf23993eb467c0a6da5a12761d","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        if not provider_dicts:"},{"line_number":240,"context_line":"            return"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        ParentAndDict \u003d collections.namedtuple(\u0027ParentAndDict\u0027,"},{"line_number":243,"context_line":"                                               [\u0027parent_uuid\u0027, \u0027pdict\u0027])"},{"line_number":244,"context_line":"        # Map of provider UUID to (parent UUID, provider dict) for the"},{"line_number":245,"context_line":"        # providers we\u0027re *adding* via this method."},{"line_number":246,"context_line":"        to_add_by_uuid \u003d {"}],"source_content_type":"text/x-python","patch_set":19,"id":"9f91af0f_264fbb93","line":243,"range":{"start_line":242,"start_character":0,"end_line":243,"end_character":72},"updated":"2018-01-10 07:39:56.000000000","message":"This object has no more info than just a provider_dict (pdict here), and I don\u0027t think this helps us a lot to read this part. Why do we need this?","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        if not provider_dicts:"},{"line_number":240,"context_line":"            return"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        ParentAndDict \u003d collections.namedtuple(\u0027ParentAndDict\u0027,"},{"line_number":243,"context_line":"                                               [\u0027parent_uuid\u0027, \u0027pdict\u0027])"},{"line_number":244,"context_line":"        # Map of provider UUID to (parent UUID, provider dict) for the"},{"line_number":245,"context_line":"        # providers we\u0027re *adding* via this method."},{"line_number":246,"context_line":"        to_add_by_uuid \u003d {"}],"source_content_type":"text/x-python","patch_set":19,"id":"7f96bb07_ab7a5521","line":243,"range":{"start_line":242,"start_character":0,"end_line":243,"end_character":72},"in_reply_to":"9f91af0f_264fbb93","updated":"2018-01-11 19:12:06.000000000","message":"It was just to save visual noise on L257, L275, L296, and L299.  I found a way to tweak it that\u0027s prettier.  It relies on for/else to initialize a variable, which I don\u0027t love, but which is programmatically correct.","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"f1282e4d87631ecf23993eb467c0a6da5a12761d","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                                               [\u0027parent_uuid\u0027, \u0027pdict\u0027])"},{"line_number":244,"context_line":"        # Map of provider UUID to (parent UUID, provider dict) for the"},{"line_number":245,"context_line":"        # providers we\u0027re *adding* via this method."},{"line_number":246,"context_line":"        to_add_by_uuid \u003d {"},{"line_number":247,"context_line":"            pd[\u0027uuid\u0027]: ParentAndDict(pd.get(\u0027parent_provider_uuid\u0027), pd)"},{"line_number":248,"context_line":"            for pd in provider_dicts}"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        # Before we go under lock, sanity check for orphans.  Every parent UUID"},{"line_number":251,"context_line":"        # must either be None (the provider is a root), or be in the tree"}],"source_content_type":"text/x-python","patch_set":19,"id":"9f91af0f_662dc3a6","line":248,"range":{"start_line":246,"start_character":0,"end_line":248,"end_character":37},"updated":"2018-01-10 07:39:56.000000000","message":"This object has no more info than just a provider_dict (pd here) as well, but I agree a dict keyed by uuid can help us to read and write this part easily;\n\n   to_add_by_uuid \u003d {pd[\u0027uuid\u0027]: pd for pd in provider_dicts} ?","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                                               [\u0027parent_uuid\u0027, \u0027pdict\u0027])"},{"line_number":244,"context_line":"        # Map of provider UUID to (parent UUID, provider dict) for the"},{"line_number":245,"context_line":"        # providers we\u0027re *adding* via this method."},{"line_number":246,"context_line":"        to_add_by_uuid \u003d {"},{"line_number":247,"context_line":"            pd[\u0027uuid\u0027]: ParentAndDict(pd.get(\u0027parent_provider_uuid\u0027), pd)"},{"line_number":248,"context_line":"            for pd in provider_dicts}"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        # Before we go under lock, sanity check for orphans.  Every parent UUID"},{"line_number":251,"context_line":"        # must either be None (the provider is a root), or be in the tree"}],"source_content_type":"text/x-python","patch_set":19,"id":"7f96bb07_2b940537","line":248,"range":{"start_line":246,"start_character":0,"end_line":248,"end_character":37},"in_reply_to":"9f91af0f_662dc3a6","updated":"2018-01-11 19:12:06.000000000","message":"Yeah","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e674a322a978ead451bb810573da2200b583e20c","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def populate_from_iterable(self, provider_dicts):"},{"line_number":249,"context_line":"        \"\"\"Populates this ProviderTree from an iterable of provider dicts."},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        :param provider_dicts: An iterable of dicts of resource provider"},{"line_number":252,"context_line":"                               information.  If a provider is present in"},{"line_number":253,"context_line":"                               provider_dicts, all its descendants must also be"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_2a90944e","line":250,"updated":"2018-01-19 17:14:22.000000000","message":"Please add a note in the docstring that explicitly states that this method will ADD providers to the tree if provider_dicts contains providers that do not exist in the tree already and will REPLACE providers in the tree if provider_dicts contains providers that are already in the tree. This method will NOT remove providers from the tree that are not in provider_dicts.","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5750e4d80fd79ba99376f08f2379b84b5396551e","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def populate_from_iterable(self, provider_dicts):"},{"line_number":249,"context_line":"        \"\"\"Populates this ProviderTree from an iterable of provider dicts."},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        :param provider_dicts: An iterable of dicts of resource provider"},{"line_number":252,"context_line":"                               information.  If a provider is present in"},{"line_number":253,"context_line":"                               provider_dicts, all its descendants must also be"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_db90e1f0","line":250,"in_reply_to":"7f96bb07_2a90944e","updated":"2018-01-19 21:57:48.000000000","message":"Couldn\u0027t have said it better myself.  So I didn\u0027t.","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e674a322a978ead451bb810573da2200b583e20c","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                    # This should never happen - we already ensured all parents"},{"line_number":296,"context_line":"                    # exist in the tree, which means we can\u0027t have any branches"},{"line_number":297,"context_line":"                    # that don\u0027t wind up at the root, which means we can\u0027t have"},{"line_number":298,"context_line":"                    # cycles.  But to quell the paranoia..."},{"line_number":299,"context_line":"                    raise ValueError("},{"line_number":300,"context_line":"                        _(\"Unexpectedly failed to find parents already in the\""},{"line_number":301,"context_line":"                          \"tree for any of the following: %s\") %"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_aa090496","line":298,"updated":"2018-01-19 17:14:22.000000000","message":"Well, another thread could add a new provider in between line  269 and line 281 and invalidate the conditions of the check that happens on line 276. I mean, yeah, there\u0027s a very minute chance of that happening (as you allude to here) but it *could* happen. Just sayin\u0027 :)\n\nThe alternative would be to just do the check inside the locked segment and create a _get_provider_uuids_with_lock() method that didn\u0027t attempt to re-lock. But I think that\u0027s overkill really.","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5750e4d80fd79ba99376f08f2379b84b5396551e","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                    # This should never happen - we already ensured all parents"},{"line_number":296,"context_line":"                    # exist in the tree, which means we can\u0027t have any branches"},{"line_number":297,"context_line":"                    # that don\u0027t wind up at the root, which means we can\u0027t have"},{"line_number":298,"context_line":"                    # cycles.  But to quell the paranoia..."},{"line_number":299,"context_line":"                    raise ValueError("},{"line_number":300,"context_line":"                        _(\"Unexpectedly failed to find parents already in the\""},{"line_number":301,"context_line":"                          \"tree for any of the following: %s\") %"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_dba9c124","line":298,"in_reply_to":"7f96bb07_aa090496","updated":"2018-01-19 21:57:48.000000000","message":"Good point.","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"6cba856b1d21658fb907175f7c7fe57987336a66","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        This method will ADD providers to the tree if provider_dicts contains"},{"line_number":252,"context_line":"        providers that do not exist in the tree already and will REPLACE"},{"line_number":253,"context_line":"        providers in the tree if provider_dicts contains providers that are"},{"line_number":254,"context_line":"        already in the tree. This method will NOT remove providers from the"},{"line_number":255,"context_line":"        tree that are not in provider_dicts."},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        :param provider_dicts: An iterable of dicts of resource provider"},{"line_number":258,"context_line":"                               information.  If a provider is present in"}],"source_content_type":"text/x-python","patch_set":30,"id":"7f96bb07_6e72107f","line":255,"range":{"start_line":254,"start_character":29,"end_line":255,"end_character":44},"updated":"2018-01-22 14:02:08.000000000","message":"...basically?  If we\u0027re not afraid of being verbose, it is good to complement here something like;\n\n\"But if a parent provider is in provider_dicts and the descendents are not, this method will remove the descendents from the tree.\"","commit_id":"33d0c96347bdb6d48a1220c03bbb5389f70d7461"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4b5e990c05a93f9c94f7cef5193a9aa1094f40a7","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        This method will ADD providers to the tree if provider_dicts contains"},{"line_number":252,"context_line":"        providers that do not exist in the tree already and will REPLACE"},{"line_number":253,"context_line":"        providers in the tree if provider_dicts contains providers that are"},{"line_number":254,"context_line":"        already in the tree. This method will NOT remove providers from the"},{"line_number":255,"context_line":"        tree that are not in provider_dicts."},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        :param provider_dicts: An iterable of dicts of resource provider"},{"line_number":258,"context_line":"                               information.  If a provider is present in"}],"source_content_type":"text/x-python","patch_set":30,"id":"5f93b717_55a3d59e","line":255,"range":{"start_line":254,"start_character":29,"end_line":255,"end_character":44},"in_reply_to":"7f96bb07_30c69b18","updated":"2018-01-30 00:00:46.000000000","message":"Done in https://review.openstack.org/#/c/531260/","commit_id":"33d0c96347bdb6d48a1220c03bbb5389f70d7461"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b0c7bb1ee20e4ab63ab7396d0c05aa2f5db6da5a","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        This method will ADD providers to the tree if provider_dicts contains"},{"line_number":252,"context_line":"        providers that do not exist in the tree already and will REPLACE"},{"line_number":253,"context_line":"        providers in the tree if provider_dicts contains providers that are"},{"line_number":254,"context_line":"        already in the tree. This method will NOT remove providers from the"},{"line_number":255,"context_line":"        tree that are not in provider_dicts."},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        :param provider_dicts: An iterable of dicts of resource provider"},{"line_number":258,"context_line":"                               information.  If a provider is present in"}],"source_content_type":"text/x-python","patch_set":30,"id":"7f96bb07_30c69b18","line":255,"range":{"start_line":254,"start_character":29,"end_line":255,"end_character":44},"in_reply_to":"7f96bb07_6e72107f","updated":"2018-01-22 15:35:31.000000000","message":"Sure, agreed. Eric can add in the accumulated nits patch later.","commit_id":"33d0c96347bdb6d48a1220c03bbb5389f70d7461"}],"nova/tests/unit/compute/test_provider_tree.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4ae42d33293c891c50539e869088f9b1b2218f10","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                expected_uuids |\u003d set([uuids.grandchild1_1, uuids.new_root])"},{"line_number":228,"context_line":"            self.assertEqual(expected_uuids, pt.uuid_set())"},{"line_number":229,"context_line":"            # Now make sure they\u0027re in the right hierarchy"},{"line_number":230,"context_line":"            root \u003d pt.find(uuids.root)"},{"line_number":231,"context_line":"            self.assertEqual(uuids.root, root.uuid)"},{"line_number":232,"context_line":"            self.assertEqual(\u0027root\u0027, root.name)"},{"line_number":233,"context_line":"            self.assertEqual(0, root.generation)"}],"source_content_type":"text/x-python","patch_set":7,"id":"df87a7cf_16149c4d","line":230,"updated":"2017-12-05 17:01:15.000000000","message":"Obviously, none of these tests are going to work now, since we no longer have a ProviderTree.find() method that returns an object, only a UUID...","commit_id":"bbab7c4e56cea0170cdb644fe144e956d4c7319a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d4650be92fc7ccc27373a80535cd9eaf8a73f1fb","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                expected_uuids |\u003d set([uuids.grandchild1_1, uuids.new_root])"},{"line_number":220,"context_line":"            self.assertEqual(expected_uuids, pt.get_provider_uuids())"},{"line_number":221,"context_line":"            # Now make sure they\u0027re in the right hierarchy"},{"line_number":222,"context_line":"            root \u003d pt.find(uuids.root)"},{"line_number":223,"context_line":"            self.assertEqual(uuids.root, root.uuid)"},{"line_number":224,"context_line":"            self.assertEqual(\u0027root\u0027, root.name)"},{"line_number":225,"context_line":"            self.assertEqual(0, root.generation)"}],"source_content_type":"text/x-python","patch_set":8,"id":"df87a7cf_a3c79445","line":222,"range":{"start_line":222,"start_character":22,"end_line":222,"end_character":26},"updated":"2017-12-06 14:17:50.000000000","message":"here\n\nI think this whole thing needs to be reworked.  May want to consider porting _validate_provider from test_report.py","commit_id":"7f3e132f62ded1162ddf4b7371cc94b10b7d1ac3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        self.assertFalse(pt.exists(numa_cell0_uuid))"},{"line_number":122,"context_line":"        self.assertFalse(pt.exists(uuids.cn1))"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def test_populate_from_iterable(self):"},{"line_number":125,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":126,"context_line":"        # Empty list is a no-op"},{"line_number":127,"context_line":"        pt.populate_from_iterable([])"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_c80c1143","line":124,"updated":"2017-12-07 16:35:48.000000000","message":"I would split this big test case to a list of small ones:\n- test_populate_from_empty\n- test_populate_with_and_orphan\n- test_populate_with_a_cycle\n- test_populate_with_a_complex_tree","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        self.assertFalse(pt.exists(numa_cell0_uuid))"},{"line_number":122,"context_line":"        self.assertFalse(pt.exists(uuids.cn1))"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def test_populate_from_iterable(self):"},{"line_number":125,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":126,"context_line":"        # Empty list is a no-op"},{"line_number":127,"context_line":"        pt.populate_from_iterable([])"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_88975f91","line":124,"in_reply_to":"df87a7cf_c80c1143","updated":"2018-01-11 19:12:06.000000000","message":"Done.  Though that last one ain\u0027t gonna be \"small\" no matter how you slice it.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                # Explicitly cheat, using private methods to access private/"},{"line_number":214,"context_line":"                # hidden _Provider members for validation.  Don\u0027t try this at"},{"line_number":215,"context_line":"                # home."},{"line_number":216,"context_line":"                with ptree.lock:"},{"line_number":217,"context_line":"                    return ptree._find_with_lock(uuid)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"            # Make sure we have all and only the expected providers"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_1378ee8d","line":216,"updated":"2017-12-07 16:35:48.000000000","message":"We have our own ptree object for the test shared with nobody so I guess we can cheat away the lock as well.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                # Explicitly cheat, using private methods to access private/"},{"line_number":214,"context_line":"                # hidden _Provider members for validation.  Don\u0027t try this at"},{"line_number":215,"context_line":"                # home."},{"line_number":216,"context_line":"                with ptree.lock:"},{"line_number":217,"context_line":"                    return ptree._find_with_lock(uuid)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"            # Make sure we have all and only the expected providers"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_486397dd","line":216,"in_reply_to":"df87a7cf_1378ee8d","updated":"2018-01-11 19:12:06.000000000","message":"Fair enough.  Done.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                uuids.ggc1_2_1, uuids.ggc1_2_2, uuids.ggc1_2_3,"},{"line_number":223,"context_line":"                uuids.another_root"},{"line_number":224,"context_line":"            ])"},{"line_number":225,"context_line":"            if test \u003d\u003d 2:"},{"line_number":226,"context_line":"                expected_uuids |\u003d set([uuids.grandchild1_1, uuids.new_root])"},{"line_number":227,"context_line":"            self.assertEqual(expected_uuids, pt.get_provider_uuids())"},{"line_number":228,"context_line":"            # Now make sure they\u0027re in the right hierarchy"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_73b37a4e","line":225,"updated":"2017-12-07 16:35:48.000000000","message":"This also triggers my urge to split this into multiple test cases with shared utils to avoid some duplication.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                uuids.ggc1_2_1, uuids.ggc1_2_2, uuids.ggc1_2_3,"},{"line_number":223,"context_line":"                uuids.another_root"},{"line_number":224,"context_line":"            ])"},{"line_number":225,"context_line":"            if test \u003d\u003d 2:"},{"line_number":226,"context_line":"                expected_uuids |\u003d set([uuids.grandchild1_1, uuids.new_root])"},{"line_number":227,"context_line":"            self.assertEqual(expected_uuids, pt.get_provider_uuids())"},{"line_number":228,"context_line":"            # Now make sure they\u0027re in the right hierarchy"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_08a10faa","line":225,"in_reply_to":"df87a7cf_73b37a4e","updated":"2018-01-11 19:12:06.000000000","message":"I don\u0027t disagree with you philosophically, but I think that would wind up being pretty ugly.  Especially for the variations in the loop starting at L235.  Let me know if you feel strongly about this.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":255,"context_line":"                self.assertEqual(\u0027new_root\u0027, new_root.name)"},{"line_number":256,"context_line":"                self.assertEqual(42, new_root.generation)"},{"line_number":257,"context_line":"                self.assertIsNone(new_root.parent_uuid)"},{"line_number":258,"context_line":"                self.assertEqual(0, len(list(new_root.children)))"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        validate_root(1)"},{"line_number":261,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_73685a86","line":258,"updated":"2017-12-07 16:35:48.000000000","message":"Does the assert of the addition of grandchild1_1 missing in test 2?","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":255,"context_line":"                self.assertEqual(\u0027new_root\u0027, new_root.name)"},{"line_number":256,"context_line":"                self.assertEqual(42, new_root.generation)"},{"line_number":257,"context_line":"                self.assertIsNone(new_root.parent_uuid)"},{"line_number":258,"context_line":"                self.assertEqual(0, len(list(new_root.children)))"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        validate_root(1)"},{"line_number":261,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_6818d31c","line":258,"in_reply_to":"df87a7cf_73685a86","updated":"2018-01-11 19:12:06.000000000","message":"That happens on L226 (we verify that the UUID is in the tree), L238 (for test 2 we verify that child has 2 children), L240 (both children\u0027s names start with \u0027grandchild1_\u0027).  I suppose I could assert that grandchild1_1 doesn\u0027t have any children of its own there too...","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e9335f153d92e8bf5fc87be3eff10bc217158cc","unresolved":false,"context_lines":[{"line_number":286,"context_line":"        # Merge an empty list - still a no-op"},{"line_number":287,"context_line":"        pt.populate_from_iterable([])"},{"line_number":288,"context_line":"        validate_root(2)"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def test_has_inventory_changed_no_existing_rp(self):"},{"line_number":291,"context_line":"        cns \u003d self.compute_nodes"},{"line_number":292,"context_line":"        pt \u003d provider_tree.ProviderTree(cns)"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_33c3b25f","line":289,"updated":"2017-12-07 16:35:48.000000000","message":"I think we need a test case with the following setup. Populate the empty tree to have: root -\u003e child -\u003e grandchild .\nThen populate the same tree with [child] . Assert that grandchild is not orphaned in the process.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":286,"context_line":"        # Merge an empty list - still a no-op"},{"line_number":287,"context_line":"        pt.populate_from_iterable([])"},{"line_number":288,"context_line":"        validate_root(2)"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def test_has_inventory_changed_no_existing_rp(self):"},{"line_number":291,"context_line":"        cns \u003d self.compute_nodes"},{"line_number":292,"context_line":"        pt \u003d provider_tree.ProviderTree(cns)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f96bb07_a53b6436","line":289,"in_reply_to":"df87a7cf_33c3b25f","updated":"2018-01-11 19:12:06.000000000","message":"Depending what you mean by \u0027orphaned\u0027...\n\nIn the scenario you mentioned, the grandchild will disappear from the tree.  This is as designed per the docstring:\n\n               If a provider is present in\n provider_dicts, all its descendants must also be\n present.\n\nBut yes, I\u0027ll add a test case for that.","commit_id":"234d3fecfc4a36a4c35473caa2d403c275727276"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"f1282e4d87631ecf23993eb467c0a6da5a12761d","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        self.assertFalse(pt.exists(pf1_cell0_uuid))"},{"line_number":121,"context_line":"        self.assertFalse(pt.exists(numa_cell0_uuid))"},{"line_number":122,"context_line":"        self.assertFalse(pt.exists(uuids.cn1))"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def test_populate_from_iterable(self):"},{"line_number":125,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":126,"context_line":"        # Empty list is a no-op"}],"source_content_type":"text/x-python","patch_set":19,"id":"9f91af0f_497f70df","line":123,"updated":"2018-01-10 07:39:56.000000000","message":"Gibi\u0027s comment on the test part on PS10 should be addressed. Why not breaking down this into pieces?","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        self.assertFalse(pt.exists(pf1_cell0_uuid))"},{"line_number":121,"context_line":"        self.assertFalse(pt.exists(numa_cell0_uuid))"},{"line_number":122,"context_line":"        self.assertFalse(pt.exists(uuids.cn1))"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def test_populate_from_iterable(self):"},{"line_number":125,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":126,"context_line":"        # Empty list is a no-op"}],"source_content_type":"text/x-python","patch_set":19,"id":"7f96bb07_4bacf18b","line":123,"in_reply_to":"9f91af0f_497f70df","updated":"2018-01-11 19:12:06.000000000","message":"Thanks, I had missed those comments.","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"f1282e4d87631ecf23993eb467c0a6da5a12761d","unresolved":false,"context_lines":[{"line_number":292,"context_line":"        # tree that\u0027s already populated.  This tests the case where a given"},{"line_number":293,"context_line":"        # provider exists both in the tree and in the input.  We must replace"},{"line_number":294,"context_line":"        # that provider *before* we inject its descendants; otherwise the"},{"line_number":295,"context_line":"        # descendants will be lost.  Note that this test case is not 100%"},{"line_number":296,"context_line":"        # reliable, as we can\u0027t predict the order over which hashed values are"},{"line_number":297,"context_line":"        # iterated."},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9f91af0f_699d9464","line":297,"range":{"start_line":295,"start_character":37,"end_line":297,"end_character":19},"updated":"2018-01-10 07:39:56.000000000","message":"If I \u0027m reading this correctly, this comment is not needed anymore?","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6d34c3a93748b6a35a135061c6e92af922b1ea3e","unresolved":false,"context_lines":[{"line_number":292,"context_line":"        # tree that\u0027s already populated.  This tests the case where a given"},{"line_number":293,"context_line":"        # provider exists both in the tree and in the input.  We must replace"},{"line_number":294,"context_line":"        # that provider *before* we inject its descendants; otherwise the"},{"line_number":295,"context_line":"        # descendants will be lost.  Note that this test case is not 100%"},{"line_number":296,"context_line":"        # reliable, as we can\u0027t predict the order over which hashed values are"},{"line_number":297,"context_line":"        # iterated."},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7f96bb07_4bdab117","line":297,"range":{"start_line":295,"start_character":37,"end_line":297,"end_character":19},"in_reply_to":"9f91af0f_699d9464","updated":"2018-01-11 19:12:06.000000000","message":"No, it\u0027s definitely needed.  In the main loop, we can\u0027t predict what order .items() gives us.  If someone tweaks the algorithm and runs this once and it succeeds, it doesn\u0027t mean the algorithm is correct.\n\nI would have loved to sort the items in some kind of breadth-first hierarchical order, but of course that would require determining that hierarchy first (chicken/egg).","commit_id":"ff2a60a46132b866d49afc15d4f8419240226f97"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e674a322a978ead451bb810573da2200b583e20c","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            \u0027uuid\u0027: uuids.child1,"},{"line_number":160,"context_line":"            \u0027name\u0027: \u0027child1\u0027,"},{"line_number":161,"context_line":"            \u0027generation\u0027: 1,"},{"line_number":162,"context_line":"            # There\u0027s a country song about this"},{"line_number":163,"context_line":"            \u0027parent_provider_uuid\u0027: uuids.grandchild1_1,"},{"line_number":164,"context_line":"        }"},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_ea4aacaa","line":162,"updated":"2018-01-19 17:14:22.000000000","message":"lmao. just run this test method in reverse and ...\n\nyou get out of jail.\n\nyou get your dog back.\n\nyou get your wife back...","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e674a322a978ead451bb810573da2200b583e20c","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        ]"},{"line_number":227,"context_line":"        pt.populate_from_iterable(plist)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        def validate_root(test):"},{"line_number":230,"context_line":"            # Make sure we have all and only the expected providers"},{"line_number":231,"context_line":"            expected_uuids \u003d set(["},{"line_number":232,"context_line":"                uuids.root, uuids.child1, uuids.child2, uuids.grandchild1_2,"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_aa98241b","line":229,"range":{"start_line":229,"start_character":26,"end_line":229,"end_character":30},"updated":"2018-01-19 17:14:22.000000000","message":"-1\n\ncan you make the parameter be expected_uuids and pass in the expected UUIDs explicitly? then, instead of passing the (inscrutable) validate_root(2) below, you can be more explicit and do something like:\n\n expected_uuids |\u003d set([uuids.grandchild1_1, uuids.new_root])\n validate_root(expected_uuids)","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5750e4d80fd79ba99376f08f2379b84b5396551e","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        ]"},{"line_number":227,"context_line":"        pt.populate_from_iterable(plist)"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"        def validate_root(test):"},{"line_number":230,"context_line":"            # Make sure we have all and only the expected providers"},{"line_number":231,"context_line":"            expected_uuids \u003d set(["},{"line_number":232,"context_line":"                uuids.root, uuids.child1, uuids.child2, uuids.grandchild1_2,"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_a6605837","line":229,"range":{"start_line":229,"start_character":26,"end_line":229,"end_character":30},"in_reply_to":"7f96bb07_aa98241b","updated":"2018-01-19 21:57:48.000000000","message":"It\u0027s not quite that simple, but since both you and gibi have hated on this, I\u0027ll refactor it.","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e674a322a978ead451bb810573da2200b583e20c","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        # tree that\u0027s already populated.  This tests the case where a given"},{"line_number":313,"context_line":"        # provider exists both in the tree and in the input.  We must replace"},{"line_number":314,"context_line":"        # that provider *before* we inject its descendants; otherwise the"},{"line_number":315,"context_line":"        # descendants will be lost.  Note that this test case is not 100%"},{"line_number":316,"context_line":"        # reliable, as we can\u0027t predict the order over which hashed values are"},{"line_number":317,"context_line":"        # iterated."},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        pt \u003d provider_tree.ProviderTree()"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_0ab0388b","line":316,"range":{"start_line":315,"start_character":37,"end_line":316,"end_character":18},"updated":"2018-01-19 17:14:22.000000000","message":"this worries me that we\u0027d be introducing latent test bugs that would be hard to diagnose...","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b14efc5bd1621d72dc270eba2060194172045e83","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        # tree that\u0027s already populated.  This tests the case where a given"},{"line_number":313,"context_line":"        # provider exists both in the tree and in the input.  We must replace"},{"line_number":314,"context_line":"        # that provider *before* we inject its descendants; otherwise the"},{"line_number":315,"context_line":"        # descendants will be lost.  Note that this test case is not 100%"},{"line_number":316,"context_line":"        # reliable, as we can\u0027t predict the order over which hashed values are"},{"line_number":317,"context_line":"        # iterated."},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        pt \u003d provider_tree.ProviderTree()"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_3078dbf6","line":316,"range":{"start_line":315,"start_character":37,"end_line":316,"end_character":18},"in_reply_to":"7f96bb07_062a6c8f","updated":"2018-01-22 15:30:13.000000000","message":"ack.","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"5750e4d80fd79ba99376f08f2379b84b5396551e","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        # tree that\u0027s already populated.  This tests the case where a given"},{"line_number":313,"context_line":"        # provider exists both in the tree and in the input.  We must replace"},{"line_number":314,"context_line":"        # that provider *before* we inject its descendants; otherwise the"},{"line_number":315,"context_line":"        # descendants will be lost.  Note that this test case is not 100%"},{"line_number":316,"context_line":"        # reliable, as we can\u0027t predict the order over which hashed values are"},{"line_number":317,"context_line":"        # iterated."},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        pt \u003d provider_tree.ProviderTree()"}],"source_content_type":"text/x-python","patch_set":29,"id":"7f96bb07_062a6c8f","line":316,"range":{"start_line":315,"start_character":37,"end_line":316,"end_character":18},"in_reply_to":"7f96bb07_0ab0388b","updated":"2018-01-19 21:57:48.000000000","message":"The alternative is orders of magnitude harder.  We would have to figure out how to get populate_from_iterable to process UUIDs in a predictable order, and then cook this test case to feed those UUIDs \"upside down\" (child first).  This test case (which Tetsuro wrote - see [1], [2]) will at least trigger *some* of the time if the algorithm is wrong.  It\u0027s better than nothing, and not worth the pain to make perfect.\n\n[1] https://review.openstack.org/#/c/520756/17/nova/compute/provider_tree.py@283\n[2] https://review.openstack.org/#/c/530035/","commit_id":"7ab0480ca6af70855a1d294d6434bf8586d55367"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3ac8e2b7fa57511e9ca2383fd91669de689dc94c","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            \u0027uuid\u0027: uuids.child1,"},{"line_number":160,"context_line":"            \u0027name\u0027: \u0027child1\u0027,"},{"line_number":161,"context_line":"            \u0027generation\u0027: 1,"},{"line_number":162,"context_line":"            # There\u0027s a country song about this"},{"line_number":163,"context_line":"            \u0027parent_provider_uuid\u0027: uuids.grandchild1_1,"},{"line_number":164,"context_line":"        }"},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"7f96bb07_ab11dc55","line":162,"updated":"2018-01-22 15:43:07.000000000","message":"✔","commit_id":"33d0c96347bdb6d48a1220c03bbb5389f70d7461"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b0c7bb1ee20e4ab63ab7396d0c05aa2f5db6da5a","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        pt.populate_from_iterable([child])"},{"line_number":389,"context_line":"        self.assertEqual(set([uuids.root, uuids.child]),"},{"line_number":390,"context_line":"                         pt.get_provider_uuids())"},{"line_number":391,"context_line":"        self.assertFalse(pt.exists(uuids.grandchild))"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def test_has_inventory_changed_no_existing_rp(self):"},{"line_number":394,"context_line":"        cns \u003d self.compute_nodes"}],"source_content_type":"text/x-python","patch_set":30,"id":"7f96bb07_b0b06b8d","line":391,"updated":"2018-01-22 15:35:31.000000000","message":"excellent unit tests here. ++","commit_id":"33d0c96347bdb6d48a1220c03bbb5389f70d7461"}]}