)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":10,"context_line":"resources then nova needs to communicate the resource allocation it"},{"line_number":11,"context_line":"made during the scheduling to neutron."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As a single neutron port\u0027s resource request is included in the"},{"line_number":14,"context_line":"allocation candidate query as a separate request group therefore the"},{"line_number":15,"context_line":"alloction for a port will come from a single RP. Neutron expects that"},{"line_number":16,"context_line":"this single RP uuid is communicated during the binding of the port in"},{"line_number":17,"context_line":"the binding_profile."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"9fdfeff1_f731a65a","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":48},"updated":"2019-02-12 19:53:06.000000000","message":"This may work for this bp, but it\u0027s going to be insufficient for e.g. cyborg where there can be M:N request groups to PFs. (Not saying you want to change it here, just looking ahead...)\n\n[Later] Actually I\u0027m not sure I understood this sentence correctly the first time through: \"the allocation for a port will come from a single RP\" - but the inverse is not true, i.e. allocations for multiple ports might also still come from a single RP.\n\nSo I think this is okay for cyborg too.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":10,"context_line":"resources then nova needs to communicate the resource allocation it"},{"line_number":11,"context_line":"made during the scheduling to neutron."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As a single neutron port\u0027s resource request is included in the"},{"line_number":14,"context_line":"allocation candidate query as a separate request group therefore the"},{"line_number":15,"context_line":"alloction for a port will come from a single RP. Neutron expects that"},{"line_number":16,"context_line":"this single RP uuid is communicated during the binding of the port in"},{"line_number":17,"context_line":"the binding_profile."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"9fdfeff1_cb7a8d22","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":48},"in_reply_to":"9fdfeff1_f731a65a","updated":"2019-02-14 14:06:16.000000000","message":"If cyborg uses numbered request groups then this code will work as is (except missing the forbidden traits, and aggregates handling).","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":24,"context_line":"required traits in the groups, the overall allocation made for the"},{"line_number":25,"context_line":"RequestSpec and the provider summary of the RPs in the allocation."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Subsequent patches will integrate this code into the server create code"},{"line_number":28,"context_line":"path to create the mapping, then change the port binding code path to"},{"line_number":29,"context_line":"use this mapping to communicate the resource provider of the port in the"},{"line_number":30,"context_line":"binding profile."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"blueprint bandwidth-resource-provider"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"9fdfeff1_37072e85","line":30,"range":{"start_line":27,"start_character":0,"end_line":30,"end_character":16},"updated":"2019-02-12 19:53:06.000000000","message":"actually I hope subsequent patches will take advantage of whatever magic we decide to implement via [1] so we don\u0027t have to do this.\n\n[1] https://review.openstack.org/#/c/597601/","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":24,"context_line":"required traits in the groups, the overall allocation made for the"},{"line_number":25,"context_line":"RequestSpec and the provider summary of the RPs in the allocation."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Subsequent patches will integrate this code into the server create code"},{"line_number":28,"context_line":"path to create the mapping, then change the port binding code path to"},{"line_number":29,"context_line":"use this mapping to communicate the resource provider of the port in the"},{"line_number":30,"context_line":"binding profile."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"blueprint bandwidth-resource-provider"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"9fdfeff1_6baa8171","line":30,"range":{"start_line":27,"start_character":0,"end_line":30,"end_character":16},"in_reply_to":"9fdfeff1_37072e85","updated":"2019-02-14 14:06:16.000000000","message":"OK, near term patches (i.e. the patches in the series) integrates this code to the main flow of nova. But I totally agree that as soon as [1] is implemented the code in this patch is ripped out from nova.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac03b61aaf3889d6017686ee544f0e8063b40374","unresolved":false,"context_lines":[{"line_number":3016,"context_line":"        for rc, amount in port_allocation.items():"},{"line_number":3017,"context_line":"            original_allocation[rc] -\u003d amount"},{"line_number":3018,"context_line":""},{"line_number":3019,"context_line":"    def _find_resource_providers_for_ports("},{"line_number":3020,"context_line":"            self, remaining_ports, remaining_allocations, provider_traits,"},{"line_number":3021,"context_line":"            port_providers):"},{"line_number":3022,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_f2b33dea","line":3019,"range":{"start_line":3019,"start_character":8,"end_line":3019,"end_character":42},"updated":"2018-11-18 11:50:15.000000000","message":"This is a pretty complicated recursive search. I can try another approach. Generate every possible port - RP mapping then filter out the invalid ones. That algorithm would probably do more steps but it would be an simpler iterative code.","commit_id":"f540b3b02318c9d2658a3feb270b9021539d713f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ac03b61aaf3889d6017686ee544f0e8063b40374","unresolved":false,"context_lines":[{"line_number":3074,"context_line":"                # To be able to do that by continuing the loop we need to"},{"line_number":3075,"context_line":"                # rollback the effect of the"},{"line_number":3076,"context_line":"                # _consume_port_allocation_from_allocations() call"},{"line_number":3077,"context_line":"                remaining_allocations \u003d orig_allocs"},{"line_number":3078,"context_line":"                continue"},{"line_number":3079,"context_line":""},{"line_number":3080,"context_line":"        # if we reached this point then non of the alternative RP mappings"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_93a30845","line":3077,"updated":"2018-11-18 11:50:15.000000000","message":"Here we need to roll back the effect of \n\n  port_providers[port_uuid] \u003d rp\n\nIt works now because port_providers is not used to decide if we found a solution or not and every port needs to be part of the solution so eventually a wrong decision in port_providers will be overwritten by the good choice of the solution.","commit_id":"f540b3b02318c9d2658a3feb270b9021539d713f"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"cebeace753fdbef10a55393cf9dc9b5c823518a6","unresolved":false,"context_lines":[{"line_number":3030,"context_line":"        instance, each port\u0027s resource request, and the traits of the RPs in"},{"line_number":3031,"context_line":"        the allocation."},{"line_number":3032,"context_line":""},{"line_number":3033,"context_line":"        This is a workaround as placement does not return which PR fulfills"},{"line_number":3034,"context_line":"        which granular request group in the allocation candidate request. There"},{"line_number":3035,"context_line":"        is a spec proposing a solution in placement:"},{"line_number":3036,"context_line":"        https://review.openstack.org/#/c/597601/"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_750f0d38","line":3033,"range":{"start_line":3033,"start_character":64,"end_line":3033,"end_character":66},"updated":"2018-11-19 15:18:32.000000000","message":"RP","commit_id":"69f2e957df7374a7d8bfbd30ebecfed940e17a2f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1131a0c7eeeb070f8873a5faec99a731c7e8f22b","unresolved":false,"context_lines":[{"line_number":3030,"context_line":"        instance, each port\u0027s resource request, and the traits of the RPs in"},{"line_number":3031,"context_line":"        the allocation."},{"line_number":3032,"context_line":""},{"line_number":3033,"context_line":"        This is a workaround as placement does not return which PR fulfills"},{"line_number":3034,"context_line":"        which granular request group in the allocation candidate request. There"},{"line_number":3035,"context_line":"        is a spec proposing a solution in placement:"},{"line_number":3036,"context_line":"        https://review.openstack.org/#/c/597601/"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_d0ad9234","line":3033,"range":{"start_line":3033,"start_character":64,"end_line":3033,"end_character":66},"in_reply_to":"3f79a3b5_750f0d38","updated":"2018-11-22 11:48:30.000000000","message":"Done","commit_id":"69f2e957df7374a7d8bfbd30ebecfed940e17a2f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d12ca4ec106d220c29f0f6371701231001d96aec","unresolved":false,"context_lines":[{"line_number":3096,"context_line":"            if self._is_valid_port_rp_mapping("},{"line_number":3097,"context_line":"                    mapping, ports_with_resource_request,"},{"line_number":3098,"context_line":"                    placement_allocations, provider_traits):"},{"line_number":3099,"context_line":"                LOG.debug(\u0027Found valid port - RP mapping %s\u0027, list(mapping))"},{"line_number":3100,"context_line":"                return dict(mapping)"},{"line_number":3101,"context_line":""},{"line_number":3102,"context_line":"        # if we reached this point then none of the possible mappings was"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_5237914d","line":3099,"range":{"start_line":3099,"start_character":62,"end_line":3099,"end_character":66},"updated":"2018-11-18 12:22:04.000000000","message":"unnecessary","commit_id":"69f2e957df7374a7d8bfbd30ebecfed940e17a2f"}],"nova/objects/request_spec.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        # is not a solution"},{"line_number":690,"context_line":"        allocs \u003d copy.deepcopy(placement_allocations)"},{"line_number":691,"context_line":"        for group, rp_uuid in mapping:"},{"line_number":692,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":693,"context_line":"            for rc, amount in group.resources.items():"},{"line_number":694,"context_line":"                if rc in rp_allocs:"},{"line_number":695,"context_line":"                    rp_allocs[rc] -\u003d amount"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_f7e686b0","line":692,"range":{"start_line":692,"start_character":24,"end_line":692,"end_character":52},"updated":"2019-02-12 19:53:06.000000000","message":"Isn\u0027t this where you need to do the copy? Otherwise you\u0027re consuming the resources once for every group you check. I think that\u0027s working in most of your test cases because you don\u0027t have more than two candidates. Not yet sure how it\u0027s working on the one with three...\n\n[Later] Or maybe I was just confused about the scope of what a \u0027mapping\u0027 is. It\u0027s the list of all the rg-to-rp mappings for the whole allocation; so we actually *do* want to decrement cumulatively as we go.\n\nTL;DR: I think this is copacetic.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"525115fa9b908cfa9548c3ad1d6c17a78c958600","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        # is not a solution"},{"line_number":690,"context_line":"        allocs \u003d copy.deepcopy(placement_allocations)"},{"line_number":691,"context_line":"        for group, rp_uuid in mapping:"},{"line_number":692,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":693,"context_line":"            for rc, amount in group.resources.items():"},{"line_number":694,"context_line":"                if rc in rp_allocs:"},{"line_number":695,"context_line":"                    rp_allocs[rc] -\u003d amount"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_4378c18e","line":692,"range":{"start_line":692,"start_character":24,"end_line":692,"end_character":52},"in_reply_to":"9fdfeff1_40c3dced","updated":"2019-02-14 18:08:06.000000000","message":"✔","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        # is not a solution"},{"line_number":690,"context_line":"        allocs \u003d copy.deepcopy(placement_allocations)"},{"line_number":691,"context_line":"        for group, rp_uuid in mapping:"},{"line_number":692,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":693,"context_line":"            for rc, amount in group.resources.items():"},{"line_number":694,"context_line":"                if rc in rp_allocs:"},{"line_number":695,"context_line":"                    rp_allocs[rc] -\u003d amount"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_40c3dced","line":692,"range":{"start_line":692,"start_character":24,"end_line":692,"end_character":52},"in_reply_to":"9fdfeff1_f7e686b0","updated":"2019-02-14 14:06:16.000000000","message":"_is_valid_group_rp_mapping() decides if one candidate request groups - RPs mapping is valid so here we want to cumulatively decrement the allocations as these allocates needs to live together.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":717,"context_line":"          supported."},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"        We can live with these limitations today as Neutron does not use"},{"line_number":720,"context_line":"        forbidden traits and aggregates in the request and each Neutron port is"},{"line_number":721,"context_line":"        mapped to a numbered group."},{"line_number":722,"context_line":""},{"line_number":723,"context_line":"        This is a workaround as placement does not return which RP fulfills"},{"line_number":724,"context_line":"        which granular request group in the allocation candidate request. There"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_5a53f717","line":721,"range":{"start_line":720,"start_character":59,"end_line":721,"end_character":35},"updated":"2019-02-12 19:53:06.000000000","message":"note to self: in cyborg terms, the Neutron port corresponds to a PF, which may provide multiple accelerators if a) they were requested in the same group, and/or b) they were requested in separate groups with group_policy\u003dnone.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"7a9b6c1f9030078e795a084bba2bbb583bd39ca8","unresolved":false,"context_lines":[{"line_number":717,"context_line":"          supported."},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"        We can live with these limitations today as Neutron does not use"},{"line_number":720,"context_line":"        forbidden traits and aggregates in the request and each Neutron port is"},{"line_number":721,"context_line":"        mapped to a numbered group."},{"line_number":722,"context_line":""},{"line_number":723,"context_line":"        This is a workaround as placement does not return which RP fulfills"},{"line_number":724,"context_line":"        which granular request group in the allocation candidate request. There"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_6f05b60f","line":721,"range":{"start_line":720,"start_character":59,"end_line":721,"end_character":35},"in_reply_to":"9fdfeff1_5a53f717","updated":"2019-02-14 05:57:42.000000000","message":"Cyborg doesn\u0027t model a PF as providing accelerators, to avoid being PCI-centric. But this point is valid: a single request group in a device profile, which would correspond to a single RG here, can ask for multiple accelerator resources.\n\nIt seems that a port_resource_request corresponds to a single port in Neutron [1]., i.e., port:RG \u003d\u003d 1:1. This is unlike the Cyborg case where accelerators:RG \u003d N:1. \n\n[1] https://git.openstack.org/cgit/openstack/nova/tree/nova/objects/request_spec.py?h\u003drefs/changes/41/625941/7#n829","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":717,"context_line":"          supported."},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"        We can live with these limitations today as Neutron does not use"},{"line_number":720,"context_line":"        forbidden traits and aggregates in the request and each Neutron port is"},{"line_number":721,"context_line":"        mapped to a numbered group."},{"line_number":722,"context_line":""},{"line_number":723,"context_line":"        This is a workaround as placement does not return which RP fulfills"},{"line_number":724,"context_line":"        which granular request group in the allocation candidate request. There"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_006e34ff","line":721,"range":{"start_line":720,"start_character":59,"end_line":721,"end_character":35},"in_reply_to":"9fdfeff1_6f05b60f","updated":"2019-02-14 14:06:16.000000000","message":"A neutron port can correspond to many things. A VF (if port.vnic_type\u003d\u003ddirect) a PF (if port.vnic_type\u003d\u003ddirect_physical) an ovs vif (if port.vnic_type\u003dnormal) and many other things I\u0027m not aware of.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":"        :param placement_allocations: The overall allocation made by the"},{"line_number":729,"context_line":"                                      scheduler for this RequestSpec"},{"line_number":730,"context_line":"        :param provider_traits: A dict keyed by resource provider uuids"},{"line_number":731,"context_line":"                                containing the list of traits the given RP has."},{"line_number":732,"context_line":"                                This dict contains info only about RPs"},{"line_number":733,"context_line":"                                appearing in the placement_allocations param."},{"line_number":734,"context_line":"        \"\"\""},{"line_number":735,"context_line":"        if \u0027requested_resources\u0027 not in self or not self.requested_resources:"},{"line_number":736,"context_line":"            # Nothing to do, so let\u0027s return early"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_97285a97","line":733,"range":{"start_line":730,"start_character":0,"end_line":733,"end_character":77},"updated":"2019-02-12 19:53:06.000000000","message":"To support aggregates, we would need another arg, presumably a mapping of rp_uuid to list(agg_uuids). Alternatively, this arg could be a thing that can hold both pieces of information. Seems ProviderTree would fit the bill nicely; can we reuse that?\n\n(Note to self: to support forbidden traits, we don\u0027t need extra information in this arg; we use the traits from here against the forbidden traits from the request groups.)","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":"        :param placement_allocations: The overall allocation made by the"},{"line_number":729,"context_line":"                                      scheduler for this RequestSpec"},{"line_number":730,"context_line":"        :param provider_traits: A dict keyed by resource provider uuids"},{"line_number":731,"context_line":"                                containing the list of traits the given RP has."},{"line_number":732,"context_line":"                                This dict contains info only about RPs"},{"line_number":733,"context_line":"                                appearing in the placement_allocations param."},{"line_number":734,"context_line":"        \"\"\""},{"line_number":735,"context_line":"        if \u0027requested_resources\u0027 not in self or not self.requested_resources:"},{"line_number":736,"context_line":"            # Nothing to do, so let\u0027s return early"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_6b1a816f","line":733,"range":{"start_line":730,"start_character":0,"end_line":733,"end_character":77},"in_reply_to":"9fdfeff1_97285a97","updated":"2019-02-14 14:06:16.000000000","message":"ProviderTree could be used here. I feel that object is a lot more heavy weight than what we need here. E.g. that object does locking and it support manipulation of the tree. \n\nAlso I consider the code in this patch temporary until https://review.openstack.org/#/c/597601/ implemented in placement (hopefully in Train). I assume we will not need aggregate for neutron port in the near future. \n\nBottom line, If it happens that we need aggregate matching here, before the placement a_c enhancement is implemented then we can consider adding ProviderTree here, but for now it seems to heavy weight.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":738,"context_line":""},{"line_number":739,"context_line":"        for group in self.requested_resources:"},{"line_number":740,"context_line":"            # See the limitations in the func doc above"},{"line_number":741,"context_line":"            if (not group.use_same_provider"},{"line_number":742,"context_line":"                    or group.aggregates"},{"line_number":743,"context_line":"                    or group.forbidden_traits):"},{"line_number":744,"context_line":"                raise NotImplementedError()"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_5a2e9792","line":741,"range":{"start_line":741,"start_character":16,"end_line":741,"end_character":43},"updated":"2019-02-12 19:53:06.000000000","message":"hm, we don\u0027t just want to ignore the unnumbered group?\n\nI mean, I guess if it had network resources on it, we should blow up. But usually it won\u0027t, right? Just VCPU, MEMORY_MB, and maybe DISK_GB?","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":738,"context_line":""},{"line_number":739,"context_line":"        for group in self.requested_resources:"},{"line_number":740,"context_line":"            # See the limitations in the func doc above"},{"line_number":741,"context_line":"            if (not group.use_same_provider"},{"line_number":742,"context_line":"                    or group.aggregates"},{"line_number":743,"context_line":"                    or group.forbidden_traits):"},{"line_number":744,"context_line":"                raise NotImplementedError()"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_cbab6dd3","line":741,"range":{"start_line":741,"start_character":16,"end_line":741,"end_character":43},"in_reply_to":"9fdfeff1_5a2e9792","updated":"2019-02-14 14:06:16.000000000","message":"At the moment I don\u0027t know what to do with the unnumbered group as that could match to more than one RPs. The code in this patch cannot handle that. So if this code sees an unnumbered group it has to raise as it cannot handle it.\n\nAt the moment nova does not add the unnumbered group to the RequestSpec as RequestGroup. It wasn\u0027t needed for me so I did not added that code. The Neutron related RequestGroups are added here [1], [2]. See also the TODO about unnumbered group top of [2].\n\n[1] https://review.openstack.org/#/c/567268/46/nova/compute/api.py@889\n[2] https://review.openstack.org/#/c/567267/27/nova/objects/request_spec.py@477","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"525115fa9b908cfa9548c3ad1d6c17a78c958600","unresolved":false,"context_lines":[{"line_number":738,"context_line":""},{"line_number":739,"context_line":"        for group in self.requested_resources:"},{"line_number":740,"context_line":"            # See the limitations in the func doc above"},{"line_number":741,"context_line":"            if (not group.use_same_provider"},{"line_number":742,"context_line":"                    or group.aggregates"},{"line_number":743,"context_line":"                    or group.forbidden_traits):"},{"line_number":744,"context_line":"                raise NotImplementedError()"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_f61f3158","line":741,"range":{"start_line":741,"start_character":16,"end_line":741,"end_character":43},"in_reply_to":"9fdfeff1_cbab6dd3","updated":"2019-02-14 18:08:06.000000000","message":"\u003e At the moment nova does not add the unnumbered group to the\n \u003e RequestSpec as RequestGroup. It wasn\u0027t needed for me so I did not\n \u003e added that code. The Neutron related RequestGroups are added here\n \u003e [1], [2]. See also the TODO about unnumbered group top of [2].\n\nOh, wow, I am shocked by this.\n\nOkay, so what happens if the flavor requested the VCPU/MEMORY_MB/DISK_GB resources through a numbered group? From what I can tell, that group will also not appear in the request spec??","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":743,"context_line":"                    or group.forbidden_traits):"},{"line_number":744,"context_line":"                raise NotImplementedError()"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        # Iterate through every possible group - RP mappings and try to find a"},{"line_number":747,"context_line":"        # valid one. If there are more than one possible solution then it is"},{"line_number":748,"context_line":"        # enough to find one as these solutions are interchangeable from"},{"line_number":749,"context_line":"        # backend (e.g. Neutron) perspective."},{"line_number":750,"context_line":"        LOG.debug(\u0027Trying to find a valid group - RP mapping for groups %s to \u0027"},{"line_number":751,"context_line":"                  \u0027allocations %s with traits %s\u0027, self.requested_resources,"},{"line_number":752,"context_line":"                  placement_allocations, provider_traits)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_7a327bf4","line":749,"range":{"start_line":746,"start_character":0,"end_line":749,"end_character":45},"updated":"2019-02-12 19:53:06.000000000","message":"note to self: Is this true for cyborg?","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":743,"context_line":"                    or group.forbidden_traits):"},{"line_number":744,"context_line":"                raise NotImplementedError()"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        # Iterate through every possible group - RP mappings and try to find a"},{"line_number":747,"context_line":"        # valid one. If there are more than one possible solution then it is"},{"line_number":748,"context_line":"        # enough to find one as these solutions are interchangeable from"},{"line_number":749,"context_line":"        # backend (e.g. Neutron) perspective."},{"line_number":750,"context_line":"        LOG.debug(\u0027Trying to find a valid group - RP mapping for groups %s to \u0027"},{"line_number":751,"context_line":"                  \u0027allocations %s with traits %s\u0027, self.requested_resources,"},{"line_number":752,"context_line":"                  placement_allocations, provider_traits)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_0bf415f0","line":749,"range":{"start_line":746,"start_character":0,"end_line":749,"end_character":45},"in_reply_to":"9fdfeff1_6f1e1660","updated":"2019-02-14 14:06:16.000000000","message":"yes, it only assumes that 1 RG fits to 1 RP and multiple RG can fit to the same RP. If cyborg uses numbered request groups then cyborg will use the same assumption. The unnumbered group is a totally different story as there 1 RG might fulfilled by multiple RPs together.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"7a9b6c1f9030078e795a084bba2bbb583bd39ca8","unresolved":false,"context_lines":[{"line_number":743,"context_line":"                    or group.forbidden_traits):"},{"line_number":744,"context_line":"                raise NotImplementedError()"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        # Iterate through every possible group - RP mappings and try to find a"},{"line_number":747,"context_line":"        # valid one. If there are more than one possible solution then it is"},{"line_number":748,"context_line":"        # enough to find one as these solutions are interchangeable from"},{"line_number":749,"context_line":"        # backend (e.g. Neutron) perspective."},{"line_number":750,"context_line":"        LOG.debug(\u0027Trying to find a valid group - RP mapping for groups %s to \u0027"},{"line_number":751,"context_line":"                  \u0027allocations %s with traits %s\u0027, self.requested_resources,"},{"line_number":752,"context_line":"                  placement_allocations, provider_traits)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_6f1e1660","line":749,"range":{"start_line":746,"start_character":0,"end_line":749,"end_character":45},"in_reply_to":"9fdfeff1_7a327bf4","updated":"2019-02-14 05:57:42.000000000","message":"I think this approach only relies on the fact that 1 RG corresponds to 1 RP (though one RP may correspond to multiple RGs), i.e. RP:RG \u003d 1:N. That is a Placement/Nova feature, and Cyborg will fit within that.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":776,"context_line":"                                  for rps in itertools.product("},{"line_number":777,"context_line":"                                      placement_allocations.keys(),"},{"line_number":778,"context_line":"                                      repeat\u003dlen(self.requested_resources)))"},{"line_number":779,"context_line":"        for mapping in every_possible_mapping:"},{"line_number":780,"context_line":"            if self._is_valid_group_rp_mapping("},{"line_number":781,"context_line":"                    mapping, placement_allocations, provider_traits):"},{"line_number":782,"context_line":"                for group, rp in mapping:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_b7b23e93","line":779,"range":{"start_line":779,"start_character":12,"end_line":779,"end_character":19},"updated":"2019-02-12 19:53:06.000000000","message":"I think for testing purposes you need to figure out a way to make the guts of this mapping deterministically ordered. Otherwise you can\u0027t prove that your \"allocation subtracting\" math is actually working (see possible bug commented above).\n\n[Later] Okay, I talked myself out of the bug. Deterministic ordering still seems like a good idea for testing purposes, but not sure it has enough value to be worthwhile.\n\n[Later still] Yeah, so you did that in the test case. You may safely ignore this comment, which I shall retain only to track my train of thought through this patch.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":776,"context_line":"                                  for rps in itertools.product("},{"line_number":777,"context_line":"                                      placement_allocations.keys(),"},{"line_number":778,"context_line":"                                      repeat\u003dlen(self.requested_resources)))"},{"line_number":779,"context_line":"        for mapping in every_possible_mapping:"},{"line_number":780,"context_line":"            if self._is_valid_group_rp_mapping("},{"line_number":781,"context_line":"                    mapping, placement_allocations, provider_traits):"},{"line_number":782,"context_line":"                for group, rp in mapping:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_cb704d75","line":779,"range":{"start_line":779,"start_character":12,"end_line":779,"end_character":19},"in_reply_to":"9fdfeff1_b7b23e93","updated":"2019-02-14 14:06:16.000000000","message":"Totally valid train of thoughts that replicates my thought process during the implementation.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6f680fd786c5612cae449a9c1d710aaf80dd76c8","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":94,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":95,"context_line":"                                                         default\u003dNone)"},{"line_number":96,"context_line":"    }"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_b629a9e6","line":95,"range":{"start_line":93,"start_character":0,"end_line":95,"end_character":70},"updated":"2019-02-14 18:17:34.000000000","message":"What would have helped (assuming I would have noticed it) is if this had a big NOTE on it stating that it currently only houses bw-related request groups. Can you do that in a fup please?","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8bd68e7bc5a2ae9c010fb5f2e623ea40957d94a1","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":94,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":95,"context_line":"                                                         default\u003dNone)"},{"line_number":96,"context_line":"    }"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_c8da7c49","line":95,"range":{"start_line":93,"start_character":0,"end_line":95,"end_character":70},"in_reply_to":"9fdfeff1_3eb8230f","updated":"2019-02-15 13:29:45.000000000","message":"Added a NOTE.","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e3a1150e6fe81835ad3b3c94b66b2edb56e71cc5","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":94,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":95,"context_line":"                                                         default\u003dNone)"},{"line_number":96,"context_line":"    }"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_a954e7fe","line":95,"range":{"start_line":93,"start_character":0,"end_line":95,"end_character":70},"in_reply_to":"9fdfeff1_8f19e822","updated":"2019-02-14 23:22:41.000000000","message":"Well, yes, it should have been named request_groups. Has that ship sailed? Renaming a field of an OVO is a PITA IIUC - or maybe it\u0027s easy as long as it hasn\u0027t been in a release yet? (Has it?)\n\nI would expect/hope that a NOTE saying the unnumbered group isn\u0027t in here would *also* be out of date quickly. Because this should eventually (sooner rather than later) hold all of the damn request groups. Why tf wouldn\u0027t it?\n\nWhich is exactly my point: until that glorious day, there should be something prominent talking about that not being the case, and why, and what the implications are.","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b1616f3383ba3efa71cb3a127e9aa2dae75ea3a","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":94,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":95,"context_line":"                                                         default\u003dNone)"},{"line_number":96,"context_line":"    }"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_3eb8230f","line":95,"range":{"start_line":93,"start_character":0,"end_line":95,"end_character":70},"in_reply_to":"9fdfeff1_a954e7fe","updated":"2019-02-15 11:12:57.000000000","message":"Renaming to request_groups:\nAs no code in a running system can use this field yet (as that would need the later patches in this series) and this field is not persisted in the db either I think in theory this can be renamed. It would mean that I break the OVO versioning scheme as it would not bump the RequestSpec OVO to version 2.0 but do the rename without version bump. However I doubt the benefit of the rename worth the struggle.\nMy original plan was to add a field that can hold every request group (from neutron, from flavor extra_spec, from cyborg..., numbered, unnumbered). Hence the generic name. I did not started the actual refactoring in this patch series to move the flavor.extra_spec groups as that would be unrelated for the bandwidth feature itself. Similarly the VGPU patches did not do that either for the same reason I guess.\n\n\nAdding a NOTE:\nI can add a note stating that eventually we want to store all the groups in this field not just groups from Neutron ports. I think that the note stating what is in this field will drift as this place in the code does not need to be modified when groups from new source is added to this field. \nAlso I crafted the initialization of this field in a way that is explicitly refers to port related resource groups[1] and left a note[2] in that place for further refactoring (that are unrelated to the bandwidth feature).\n\n[1] https://review.openstack.org/#/c/616239/24/nova/objects/request_spec.py@425\n[2] https://review.openstack.org/#/c/616239/24/nova/objects/request_spec.py@467","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cda552ede6cc740180269ff1344a28e23d719fab","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":94,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":95,"context_line":"                                                         default\u003dNone)"},{"line_number":96,"context_line":"    }"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_8f19e822","line":95,"range":{"start_line":93,"start_character":0,"end_line":95,"end_character":70},"in_reply_to":"9fdfeff1_b629a9e6","updated":"2019-02-14 21:14:04.000000000","message":"Not sure I agree with that, Eric. Such a comment would undoubtedly become quickly out-of-date as soon as other things start using the RequestGroup functionality for requesting discrete related resources/traits.\n\nNow, if you\u0027re just pointing out that there should be a NOTE saying that this field does NOT house the unnumbered request group, then yes, I agree with that. :)\n\nAlternately, if you were suggesting that this field is improperly named and should instead be called \"request_groups\", I\u0027d agree with that even more.","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cda552ede6cc740180269ff1344a28e23d719fab","unresolved":false,"context_lines":[{"line_number":672,"context_line":"        self.obj_reset_changes([\u0027force_hosts\u0027, \u0027force_nodes\u0027])"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"    def _is_valid_group_rp_mapping("},{"line_number":675,"context_line":"            self, mapping, placement_allocations, provider_traits):"},{"line_number":676,"context_line":"        # Check that traits are matching for each group - rp pair in"},{"line_number":677,"context_line":"        # this mapping"},{"line_number":678,"context_line":"        for group, rp_uuid in mapping:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_6f94c437","line":675,"range":{"start_line":675,"start_character":18,"end_line":675,"end_character":25},"updated":"2019-02-14 21:14:04.000000000","message":"mapping is incredibly generic. Can we please a) rename it to something more descriptive and b) add a docstring that indicates the structure of the parameters for this function?","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8bd68e7bc5a2ae9c010fb5f2e623ea40957d94a1","unresolved":false,"context_lines":[{"line_number":672,"context_line":"        self.obj_reset_changes([\u0027force_hosts\u0027, \u0027force_nodes\u0027])"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"    def _is_valid_group_rp_mapping("},{"line_number":675,"context_line":"            self, mapping, placement_allocations, provider_traits):"},{"line_number":676,"context_line":"        # Check that traits are matching for each group - rp pair in"},{"line_number":677,"context_line":"        # this mapping"},{"line_number":678,"context_line":"        for group, rp_uuid in mapping:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_c896fc3e","line":675,"range":{"start_line":675,"start_character":18,"end_line":675,"end_character":25},"in_reply_to":"9fdfeff1_5ed16722","updated":"2019-02-15 13:29:45.000000000","message":"Done","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b1616f3383ba3efa71cb3a127e9aa2dae75ea3a","unresolved":false,"context_lines":[{"line_number":672,"context_line":"        self.obj_reset_changes([\u0027force_hosts\u0027, \u0027force_nodes\u0027])"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"    def _is_valid_group_rp_mapping("},{"line_number":675,"context_line":"            self, mapping, placement_allocations, provider_traits):"},{"line_number":676,"context_line":"        # Check that traits are matching for each group - rp pair in"},{"line_number":677,"context_line":"        # this mapping"},{"line_number":678,"context_line":"        for group, rp_uuid in mapping:"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_5ed16722","line":675,"range":{"start_line":675,"start_character":18,"end_line":675,"end_character":25},"in_reply_to":"9fdfeff1_6f94c437","updated":"2019-02-15 11:12:57.000000000","message":"Good point. Let me do both a) and b).","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cda552ede6cc740180269ff1344a28e23d719fab","unresolved":false,"context_lines":[{"line_number":781,"context_line":"                    mapping, placement_allocations, provider_traits):"},{"line_number":782,"context_line":"                for group, rp in mapping:"},{"line_number":783,"context_line":"                    # NOTE(gibi): un-numbered group might be mapped to more"},{"line_number":784,"context_line":"                    # than on RP but we does not support that yet here."},{"line_number":785,"context_line":"                    group.provider_uuids \u003d [rp]"},{"line_number":786,"context_line":"                LOG.debug(\u0027Found valid group - RP mapping %s\u0027, mapping)"},{"line_number":787,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_ef9f544d","line":784,"range":{"start_line":784,"start_character":40,"end_line":784,"end_character":44},"updated":"2019-02-14 21:14:04.000000000","message":"do","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cda552ede6cc740180269ff1344a28e23d719fab","unresolved":false,"context_lines":[{"line_number":781,"context_line":"                    mapping, placement_allocations, provider_traits):"},{"line_number":782,"context_line":"                for group, rp in mapping:"},{"line_number":783,"context_line":"                    # NOTE(gibi): un-numbered group might be mapped to more"},{"line_number":784,"context_line":"                    # than on RP but we does not support that yet here."},{"line_number":785,"context_line":"                    group.provider_uuids \u003d [rp]"},{"line_number":786,"context_line":"                LOG.debug(\u0027Found valid group - RP mapping %s\u0027, mapping)"},{"line_number":787,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_cfa4d0a8","line":784,"range":{"start_line":784,"start_character":27,"end_line":784,"end_character":29},"updated":"2019-02-14 21:14:04.000000000","message":"one","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8bd68e7bc5a2ae9c010fb5f2e623ea40957d94a1","unresolved":false,"context_lines":[{"line_number":781,"context_line":"                    mapping, placement_allocations, provider_traits):"},{"line_number":782,"context_line":"                for group, rp in mapping:"},{"line_number":783,"context_line":"                    # NOTE(gibi): un-numbered group might be mapped to more"},{"line_number":784,"context_line":"                    # than on RP but we does not support that yet here."},{"line_number":785,"context_line":"                    group.provider_uuids \u003d [rp]"},{"line_number":786,"context_line":"                LOG.debug(\u0027Found valid group - RP mapping %s\u0027, mapping)"},{"line_number":787,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_48aa0c71","line":784,"range":{"start_line":784,"start_character":27,"end_line":784,"end_character":29},"in_reply_to":"9fdfeff1_cfa4d0a8","updated":"2019-02-15 13:29:45.000000000","message":"Done","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8bd68e7bc5a2ae9c010fb5f2e623ea40957d94a1","unresolved":false,"context_lines":[{"line_number":781,"context_line":"                    mapping, placement_allocations, provider_traits):"},{"line_number":782,"context_line":"                for group, rp in mapping:"},{"line_number":783,"context_line":"                    # NOTE(gibi): un-numbered group might be mapped to more"},{"line_number":784,"context_line":"                    # than on RP but we does not support that yet here."},{"line_number":785,"context_line":"                    group.provider_uuids \u003d [rp]"},{"line_number":786,"context_line":"                LOG.debug(\u0027Found valid group - RP mapping %s\u0027, mapping)"},{"line_number":787,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_a89ff84d","line":784,"range":{"start_line":784,"start_character":40,"end_line":784,"end_character":44},"in_reply_to":"9fdfeff1_ef9f544d","updated":"2019-02-15 13:29:45.000000000","message":"Done","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cda552ede6cc740180269ff1344a28e23d719fab","unresolved":false,"context_lines":[{"line_number":793,"context_line":"        raise ValueError(\u0027No valid group - RP mapping is found for \u0027"},{"line_number":794,"context_line":"                         \u0027groups %s, allocation %s and provider traits %s\u0027 %"},{"line_number":795,"context_line":"                         (self.requested_resources, placement_allocations,"},{"line_number":796,"context_line":"                          provider_traits))"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_8f2c88ac","line":796,"updated":"2019-02-14 21:14:04.000000000","message":"The above function is very difficult to follow, even with the (excellent) code comments. Maybe it\u0027s just me, but I feel the algorithm would be easier to follow if instead of doing the whole list(zip(itertools.product())) thing, you had a simple loop over the request groups (self.requested_resources) and determined the provider used for each request group within the loop.\n\nIn other words, do something like this:\n\n allocs \u003d copy.deepcopy(placement_allocations)\n for rg in self.requested_resources:\n     rp_uuid \u003d self._find_provider_for_group(rg, allocs, provider_traits)\n     if rp_uuid is None:\n         raise ValueError(...)\n     rg.provider_uuids \u003d [rp_uuid]\n\nAnd then implement _find_provider_for_group() to simply find the first resource provider that meets the request group\u0027s traits and resource constraints:\n\n def _find_provider_for_group(self, group, allocs, prov_traits):\n     # NOTE that allocs is updated to have its resource\n     # totals decremented if we find a provider having\n     # matching resources...\n     \n     for rp_uuid, alloc_dict in allocs.items():\n         # Check if the provider has all the traits this\n         # request group required... skip otherwise\n         if (not all(req_trait in prov_traits[rp_uuid]\n                 for req_trait in group.required_traits)):\n             continue\n\n         rp_allocs \u003d alloc_dict[\u0027resources\u0027]\n         grp_resources \u003d group.resources.items()\n         # Check if provider has enough capacity for each\n         # resource we\u0027re requesting in this group. If not,\n         # skip it\n         if not all(rp_allocs[rc] \u003e\u003d grp_resources[rc]\n                    for rc in group.resources):\n             continue\n         \n         # Cool, got a match. Decrement the usage counts\n         # so next iteration of _find_provider_for_group()\n         # accounts for this \"pick\"\n         for rc, amount in group.resources.items():\n             if rc in rp_allocs:\n                 rp_allocs[rc] -\u003d amount\n         return rp_uuid","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b1616f3383ba3efa71cb3a127e9aa2dae75ea3a","unresolved":false,"context_lines":[{"line_number":793,"context_line":"        raise ValueError(\u0027No valid group - RP mapping is found for \u0027"},{"line_number":794,"context_line":"                         \u0027groups %s, allocation %s and provider traits %s\u0027 %"},{"line_number":795,"context_line":"                         (self.requested_resources, placement_allocations,"},{"line_number":796,"context_line":"                          provider_traits))"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":24,"id":"9fdfeff1_7e7a2b24","line":796,"in_reply_to":"9fdfeff1_8f2c88ac","updated":"2019-02-15 11:12:57.000000000","message":"I think your proposal does not solve the actual problem. Simply finding the first RP that can fulfill a group does not mean that we found a solution for every group. Look at the test case at [1]. Or let me try to give a bit smaller example than that below:\n\nConsider the following setup with two RPs and two groups:\nRP1:\n* ING: 10\n* EGR: 1\n\nRP2:\n* ING: 1\n* EGR: 10\n\nGR1:\n* ING: 1\n* EGR: 1\n\nGR2:\n* ING:10\n* EGR: 1\n\nGR3:\n* ING: 0\n* EGR: 9\n\nYou greedy algorithm would take the first group GR1 and would select RP1 to fulfill it so RP1 would be left with ING: 9, EGR: 0. Then it would take GR2 and stop with no solution. But there is a solution, by mapping GR2 to RP1, GR1 to RP2, GR3 to RP2. I think this problem cannot be solved with greedy strategy. It can be solved with your way by introducing backtracking. So when your algo founds that GR2 cannot be mapped, it goes back and try to select another RP for GR1. I did started with such a recursive backtracking algo but the end result looked pretty messy[2]. \n\n\n[1] https://review.openstack.org/#/c/616239/24/nova/tests/unit/objects/test_request_spec.py@1305\n[2] https://review.openstack.org/#/c/616239/5/nova/network/neutronv2/api.py","commit_id":"34deb5a903ed1ad924b9cd7aecb6bb0a79bf07a3"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"21cebe3cb1f1cde024005c44eb808ba91b7f7e5b","unresolved":false,"context_lines":[{"line_number":700,"context_line":"        # Check that traits are matching for each group - rp pair in"},{"line_number":701,"context_line":"        # this mapping"},{"line_number":702,"context_line":"        for group, rp_uuid in group_rp_mapping:"},{"line_number":703,"context_line":"            if (not all(req_trait in provider_traits[rp_uuid]"},{"line_number":704,"context_line":"                        for req_trait in group.required_traits)):"},{"line_number":705,"context_line":"                return False"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":"        # TODO(gibi): add support for groups with forbidden_traits and"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_d1b76920","line":704,"range":{"start_line":703,"start_character":0,"end_line":704,"end_character":65},"updated":"2019-02-18 10:02:08.000000000","message":"nit: set arith could be used: \nlike set(provider_traits[rp_uuid]) \u0026 set( group.required_traits) !\u003d set( group.required_traits)\n\nBTW it\u0027s good to have this list as sets initially.","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e643449fcd4e8e4777d299f6be20dd87bc599d44","unresolved":false,"context_lines":[{"line_number":700,"context_line":"        # Check that traits are matching for each group - rp pair in"},{"line_number":701,"context_line":"        # this mapping"},{"line_number":702,"context_line":"        for group, rp_uuid in group_rp_mapping:"},{"line_number":703,"context_line":"            if (not all(req_trait in provider_traits[rp_uuid]"},{"line_number":704,"context_line":"                        for req_trait in group.required_traits)):"},{"line_number":705,"context_line":"                return False"},{"line_number":706,"context_line":""},{"line_number":707,"context_line":"        # TODO(gibi): add support for groups with forbidden_traits and"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_3df3240b","line":704,"range":{"start_line":703,"start_character":0,"end_line":704,"end_character":65},"in_reply_to":"9fdfeff1_d1b76920","updated":"2019-02-18 13:15:29.000000000","message":"Yeah, why not.","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"21cebe3cb1f1cde024005c44eb808ba91b7f7e5b","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        # that it is mapped to in the current mapping. Consume each group\u0027s"},{"line_number":712,"context_line":"        # request from the allocation, if anything drops below zero, then this"},{"line_number":713,"context_line":"        # is not a solution"},{"line_number":714,"context_line":"        allocs \u003d copy.deepcopy(placement_allocations)"},{"line_number":715,"context_line":"        for group, rp_uuid in group_rp_mapping:"},{"line_number":716,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":717,"context_line":"            for rc, amount in group.resources.items():"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_9109e1dd","line":714,"range":{"start_line":714,"start_character":8,"end_line":714,"end_character":53},"updated":"2019-02-18 10:02:08.000000000","message":"This can be avoided with collections.Counter\nIn [28]: Counter({\u0027a\u0027: 10, \u0027b\u0027: 20, \u0027c\u0027: 30}) - Counter({\u0027a\u0027: 5, \u0027b\u0027: 10, \u0027c\u0027: 30})\nOut[28]: Counter({\u0027a\u0027: 5, \u0027b\u0027: 10})","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"76f467466f952944cae92529d0fc5ef03036acdc","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        # that it is mapped to in the current mapping. Consume each group\u0027s"},{"line_number":712,"context_line":"        # request from the allocation, if anything drops below zero, then this"},{"line_number":713,"context_line":"        # is not a solution"},{"line_number":714,"context_line":"        allocs \u003d copy.deepcopy(placement_allocations)"},{"line_number":715,"context_line":"        for group, rp_uuid in group_rp_mapping:"},{"line_number":716,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":717,"context_line":"            for rc, amount in group.resources.items():"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_7ed73451","line":714,"range":{"start_line":714,"start_character":8,"end_line":714,"end_character":53},"in_reply_to":"9fdfeff1_1d3e608a","updated":"2019-02-20 11:06:32.000000000","message":"Was wrong about copying.\nThe thing I tried to say it\u0027s good to have a function to operate on resources, like:\nsubstract(rp_allocs, group.resources). This is nitpicking of course.","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e643449fcd4e8e4777d299f6be20dd87bc599d44","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        # that it is mapped to in the current mapping. Consume each group\u0027s"},{"line_number":712,"context_line":"        # request from the allocation, if anything drops below zero, then this"},{"line_number":713,"context_line":"        # is not a solution"},{"line_number":714,"context_line":"        allocs \u003d copy.deepcopy(placement_allocations)"},{"line_number":715,"context_line":"        for group, rp_uuid in group_rp_mapping:"},{"line_number":716,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":717,"context_line":"            for rc, amount in group.resources.items():"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_1d3e608a","line":714,"range":{"start_line":714,"start_character":8,"end_line":714,"end_character":53},"in_reply_to":"9fdfeff1_9109e1dd","updated":"2019-02-18 13:15:29.000000000","message":"Do you mean copying each placement_allocations[rp_uuid] into a counter as well as each groups resource request and do the arithmetic on those counter objects? I think that is a bit more copying than currently copying only the placement_allocations.","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c499a3963308bb7da975e069d5fbe284c8e7eb3d","unresolved":false,"context_lines":[{"line_number":721,"context_line":"                        return False"},{"line_number":722,"context_line":"                else:"},{"line_number":723,"context_line":"                    return False"},{"line_number":724,"context_line":"        # If both the traits and the allocations are OK then mapping is valid"},{"line_number":725,"context_line":"        return True"},{"line_number":726,"context_line":""},{"line_number":727,"context_line":"    def map_requested_resources_to_providers("}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_b725115e","line":724,"updated":"2019-02-15 16:18:39.000000000","message":"Have to check that all the allocation from all the RPs are consumed. Without that we can found an overlapping solution for request that used isolate.\n\nhttps://etherpad.openstack.org/p/CdbhX84nKI","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e643449fcd4e8e4777d299f6be20dd87bc599d44","unresolved":false,"context_lines":[{"line_number":721,"context_line":"                        return False"},{"line_number":722,"context_line":"                else:"},{"line_number":723,"context_line":"                    return False"},{"line_number":724,"context_line":"        # If both the traits and the allocations are OK then mapping is valid"},{"line_number":725,"context_line":"        return True"},{"line_number":726,"context_line":""},{"line_number":727,"context_line":"    def map_requested_resources_to_providers("}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_5d006801","line":724,"in_reply_to":"9fdfeff1_b725115e","updated":"2019-02-18 13:15:29.000000000","message":"If we found an overlapping solution with leftover then the leftover means placement returned an allocation candidate with more resource than what was requested. Which should not really happen (except if un-numbered group also request such RCs). I added a negative test case and a check here.","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"21cebe3cb1f1cde024005c44eb808ba91b7f7e5b","unresolved":false,"context_lines":[{"line_number":797,"context_line":"        # iterator in py3.x. Still we need to generate a mapping once hence the"},{"line_number":798,"context_line":"        # generator expression."},{"line_number":799,"context_line":"        every_possible_mapping \u003d (list(zip(self.requested_resources, rps))"},{"line_number":800,"context_line":"                                  for rps in itertools.product("},{"line_number":801,"context_line":"                                      placement_allocations.keys(),"},{"line_number":802,"context_line":"                                      repeat\u003dlen(self.requested_resources)))"},{"line_number":803,"context_line":"        for mapping in every_possible_mapping:"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_d113e9ff","line":800,"range":{"start_line":800,"start_character":45,"end_line":800,"end_character":62},"updated":"2019-02-18 10:02:08.000000000","message":"Product grows very fast and input values are user input AFAIU, so some limits should be applied.\nOverwise, malicious user can attack nova-api easily.","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"76f467466f952944cae92529d0fc5ef03036acdc","unresolved":false,"context_lines":[{"line_number":797,"context_line":"        # iterator in py3.x. Still we need to generate a mapping once hence the"},{"line_number":798,"context_line":"        # generator expression."},{"line_number":799,"context_line":"        every_possible_mapping \u003d (list(zip(self.requested_resources, rps))"},{"line_number":800,"context_line":"                                  for rps in itertools.product("},{"line_number":801,"context_line":"                                      placement_allocations.keys(),"},{"line_number":802,"context_line":"                                      repeat\u003dlen(self.requested_resources)))"},{"line_number":803,"context_line":"        for mapping in every_possible_mapping:"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_99a8f28c","line":800,"range":{"start_line":800,"start_character":45,"end_line":800,"end_character":62},"in_reply_to":"9fdfeff1_7d7a0c88","updated":"2019-02-20 11:06:32.000000000","message":"I don\u0027t see a reason why it\u0027s only neutron ports.\nIt there some filtering for neutron request groups only I missed?","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"38e84d1379f074b5009c8bd52f688c792dd15f0f","unresolved":false,"context_lines":[{"line_number":797,"context_line":"        # iterator in py3.x. Still we need to generate a mapping once hence the"},{"line_number":798,"context_line":"        # generator expression."},{"line_number":799,"context_line":"        every_possible_mapping \u003d (list(zip(self.requested_resources, rps))"},{"line_number":800,"context_line":"                                  for rps in itertools.product("},{"line_number":801,"context_line":"                                      placement_allocations.keys(),"},{"line_number":802,"context_line":"                                      repeat\u003dlen(self.requested_resources)))"},{"line_number":803,"context_line":"        for mapping in every_possible_mapping:"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_23a2f44c","line":800,"range":{"start_line":800,"start_character":45,"end_line":800,"end_character":62},"in_reply_to":"9fdfeff1_99a8f28c","updated":"2019-02-20 14:34:54.000000000","message":"This is the only place currently that adds RequestGroup object to RequestSpec. And this code is working on Neutron ports.\n\nhttps://github.com/openstack/nova/blob/3548cf59217f62966a21ea65a8cb744606431bd6/nova/objects/request_spec.py#L476","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e643449fcd4e8e4777d299f6be20dd87bc599d44","unresolved":false,"context_lines":[{"line_number":797,"context_line":"        # iterator in py3.x. Still we need to generate a mapping once hence the"},{"line_number":798,"context_line":"        # generator expression."},{"line_number":799,"context_line":"        every_possible_mapping \u003d (list(zip(self.requested_resources, rps))"},{"line_number":800,"context_line":"                                  for rps in itertools.product("},{"line_number":801,"context_line":"                                      placement_allocations.keys(),"},{"line_number":802,"context_line":"                                      repeat\u003dlen(self.requested_resources)))"},{"line_number":803,"context_line":"        for mapping in every_possible_mapping:"}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_7d7a0c88","line":800,"range":{"start_line":800,"start_character":45,"end_line":800,"end_character":62},"in_reply_to":"9fdfeff1_d113e9ff","updated":"2019-02-18 13:15:29.000000000","message":"The input is coming from neutron ports. So this input can only grow through adding more neutron ports to the create request. The number of ports already limited. Also each port needs resource request which can be fulfilled by placement before we hit this code so if the available resource are limited then this product cannot grow that high. (Also similar algorithm runs in placement to found the allocation candidates and that will see the growth of the product _before_ this code is hit)","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"21cebe3cb1f1cde024005c44eb808ba91b7f7e5b","unresolved":false,"context_lines":[{"line_number":806,"context_line":"                for group, rp in mapping:"},{"line_number":807,"context_line":"                    # NOTE(gibi): un-numbered group might be mapped to more"},{"line_number":808,"context_line":"                    # than one RP but we do not support that yet here."},{"line_number":809,"context_line":"                    group.provider_uuids \u003d [rp]"},{"line_number":810,"context_line":"                LOG.debug(\u0027Found valid group - RP mapping %s\u0027, mapping)"},{"line_number":811,"context_line":"                return"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_31afd57d","line":809,"range":{"start_line":809,"start_character":19,"end_line":809,"end_character":47},"updated":"2019-02-18 10:02:08.000000000","message":"It\u0027s hard to follow what is actually changed by this method.","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e643449fcd4e8e4777d299f6be20dd87bc599d44","unresolved":false,"context_lines":[{"line_number":806,"context_line":"                for group, rp in mapping:"},{"line_number":807,"context_line":"                    # NOTE(gibi): un-numbered group might be mapped to more"},{"line_number":808,"context_line":"                    # than one RP but we do not support that yet here."},{"line_number":809,"context_line":"                    group.provider_uuids \u003d [rp]"},{"line_number":810,"context_line":"                LOG.debug(\u0027Found valid group - RP mapping %s\u0027, mapping)"},{"line_number":811,"context_line":"                return"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"9fdfeff1_dd6a18b1","line":809,"range":{"start_line":809,"start_character":19,"end_line":809,"end_character":47},"in_reply_to":"9fdfeff1_31afd57d","updated":"2019-02-18 13:15:29.000000000","message":"This fact is documented in the first line of the function doc. \n\n\"Fill the provider_uuids field in each RequestGroup objects in the requested_resources field\"\n\nhttps://review.openstack.org/#/c/616239/26/nova/objects/request_spec.py@729","commit_id":"695dfc59f2adea9995c138a9694e6f4368fbf185"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a7d54d5a4e3e9d59e6faf7704b848cb0de0a04a3","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        # NOTE(gibi): Eventually we want to store every resource request as"},{"line_number":94,"context_line":"        # RequestGroup objects here. However currently the flavor based"},{"line_number":95,"context_line":"        # resources like vcpu, ram, disk, and flavor.extra_spec based resources"},{"line_number":96,"context_line":"        # are not handled this way. See the Todo in from_components() where"},{"line_number":97,"context_line":"        # requested_resources are set."},{"line_number":98,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":99,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":100,"context_line":"                                                         default\u003dNone)"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_462ce649","line":97,"range":{"start_line":93,"start_character":0,"end_line":97,"end_character":38},"updated":"2019-02-20 15:00:15.000000000","message":"++\n\nWould also be neat to enhance *that* TODO to point back here, since we were concerned about this comment getting stale when we start accounting for the other request groups.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        # NOTE(gibi): Eventually we want to store every resource request as"},{"line_number":94,"context_line":"        # RequestGroup objects here. However currently the flavor based"},{"line_number":95,"context_line":"        # resources like vcpu, ram, disk, and flavor.extra_spec based resources"},{"line_number":96,"context_line":"        # are not handled this way. See the Todo in from_components() where"},{"line_number":97,"context_line":"        # requested_resources are set."},{"line_number":98,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":99,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":100,"context_line":"                                                         default\u003dNone)"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_3b38c739","line":97,"range":{"start_line":93,"start_character":0,"end_line":97,"end_character":38},"in_reply_to":"9fdfeff1_462ce649","updated":"2019-02-21 09:37:42.000000000","message":"Added a backref in a fup","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \u0027security_groups\u0027: fields.ObjectField(\u0027SecurityGroupList\u0027),"},{"line_number":91,"context_line":"        \u0027network_metadata\u0027: fields.ObjectField(\u0027NetworkMetadata\u0027),"},{"line_number":92,"context_line":"        \u0027is_bfv\u0027: fields.BooleanField(),"},{"line_number":93,"context_line":"        # NOTE(gibi): Eventually we want to store every resource request as"},{"line_number":94,"context_line":"        # RequestGroup objects here. However currently the flavor based"},{"line_number":95,"context_line":"        # resources like vcpu, ram, disk, and flavor.extra_spec based resources"},{"line_number":96,"context_line":"        # are not handled this way. See the Todo in from_components() where"},{"line_number":97,"context_line":"        # requested_resources are set."},{"line_number":98,"context_line":"        \u0027requested_resources\u0027: fields.ListOfObjectsField(\u0027RequestGroup\u0027,"},{"line_number":99,"context_line":"                                                         nullable\u003dTrue,"},{"line_number":100,"context_line":"                                                         default\u003dNone)"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_ec0cd9ed","line":97,"range":{"start_line":93,"start_character":0,"end_line":97,"end_character":38},"in_reply_to":"9fdfeff1_462ce649","updated":"2019-02-20 17:45:07.000000000","message":"Thanks for adding this comment, Gibi. ++","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":684,"context_line":"        :param group_rp_mapping: A list of RequestGroup - RP UUID two tuples"},{"line_number":685,"context_line":"                                representing a mapping between request groups"},{"line_number":686,"context_line":"                                in this RequestSpec and RPs from the"},{"line_number":687,"context_line":"                                allocation. It contains every RequestGroup in"},{"line_number":688,"context_line":"                                this RequestSpec but the mapping might not be"},{"line_number":689,"context_line":"                                valid from resources and traits perspective."},{"line_number":690,"context_line":"        :param placement_allocations: The overall allocation made by the"},{"line_number":691,"context_line":"                                      scheduler for this RequestSpec"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_0c2b5d30","line":688,"range":{"start_line":687,"start_character":43,"end_line":688,"end_character":48},"updated":"2019-02-20 17:45:07.000000000","message":"well, kinda.. :) not the unnumbered request group.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a7d54d5a4e3e9d59e6faf7704b848cb0de0a04a3","unresolved":false,"context_lines":[{"line_number":694,"context_line":"                                This dict contains info only about RPs"},{"line_number":695,"context_line":"                                appearing in the placement_allocations param."},{"line_number":696,"context_line":"        :return: True if each group\u0027s resource and trait request can be"},{"line_number":697,"context_line":"                 fulfilled from RP it is mapped to. False otherwise."},{"line_number":698,"context_line":"        \"\"\""},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"        # Check that traits are matching for each group - rp pair in"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_46d1462f","line":697,"range":{"start_line":697,"start_character":32,"end_line":697,"end_character":34},"updated":"2019-02-20 15:00:15.000000000","message":"the RP","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":694,"context_line":"                                This dict contains info only about RPs"},{"line_number":695,"context_line":"                                appearing in the placement_allocations param."},{"line_number":696,"context_line":"        :return: True if each group\u0027s resource and trait request can be"},{"line_number":697,"context_line":"                 fulfilled from RP it is mapped to. False otherwise."},{"line_number":698,"context_line":"        \"\"\""},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"        # Check that traits are matching for each group - rp pair in"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_1b262318","line":697,"range":{"start_line":697,"start_character":32,"end_line":697,"end_character":34},"in_reply_to":"9fdfeff1_46d1462f","updated":"2019-02-21 09:37:42.000000000","message":"Done in a fup","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":703,"context_line":"            if (set(provider_traits[rp_uuid])"},{"line_number":704,"context_line":"                    \u0026 set(group.required_traits)"},{"line_number":705,"context_line":"                    !\u003d set(group.required_traits)):"},{"line_number":706,"context_line":"                return False"},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"        # TODO(gibi): add support for groups with forbidden_traits and"},{"line_number":709,"context_line":"        # aggregates"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_2cb661de","line":706,"updated":"2019-02-20 17:45:07.000000000","message":"✔\n\ncould also do:\n\n if not group.required_traits.issubset(provider_traits[rp_uuid]):\n     return False","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":703,"context_line":"            if (set(provider_traits[rp_uuid])"},{"line_number":704,"context_line":"                    \u0026 set(group.required_traits)"},{"line_number":705,"context_line":"                    !\u003d set(group.required_traits)):"},{"line_number":706,"context_line":"                return False"},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"        # TODO(gibi): add support for groups with forbidden_traits and"},{"line_number":709,"context_line":"        # aggregates"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_bb7a77ed","line":706,"in_reply_to":"9fdfeff1_2cb661de","updated":"2019-02-21 09:37:42.000000000","message":"Even better. Done in a fup","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":713,"context_line":"        # request from the allocation, if anything drops below zero, then this"},{"line_number":714,"context_line":"        # is not a solution"},{"line_number":715,"context_line":"        rcs \u003d set()"},{"line_number":716,"context_line":"        allocs \u003d copy.deepcopy(placement_allocations)"},{"line_number":717,"context_line":"        for group, rp_uuid in group_rp_mapping:"},{"line_number":718,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":719,"context_line":"            for rc, amount in group.resources.items():"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_d78f1e2c","line":716,"updated":"2019-02-20 17:45:07.000000000","message":"✔","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":726,"context_line":"                    return False"},{"line_number":727,"context_line":""},{"line_number":728,"context_line":"        # Check that all the allocations are consumed from the resource"},{"line_number":729,"context_line":"        # classes that are appeared in the request groups. It should never"},{"line_number":730,"context_line":"        # happen that we have a match but also have some leftover if placement"},{"line_number":731,"context_line":"        # returns valid allocation candidates. Except if the leftover in the"},{"line_number":732,"context_line":"        # allocation are due to the RC requested in the unnumbered group."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_77aaf271","line":729,"range":{"start_line":729,"start_character":23,"end_line":729,"end_character":38},"updated":"2019-02-20 17:45:07.000000000","message":"s/are appeared in/appear in/","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":726,"context_line":"                    return False"},{"line_number":727,"context_line":""},{"line_number":728,"context_line":"        # Check that all the allocations are consumed from the resource"},{"line_number":729,"context_line":"        # classes that are appeared in the request groups. It should never"},{"line_number":730,"context_line":"        # happen that we have a match but also have some leftover if placement"},{"line_number":731,"context_line":"        # returns valid allocation candidates. Except if the leftover in the"},{"line_number":732,"context_line":"        # allocation are due to the RC requested in the unnumbered group."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_3b668757","line":729,"range":{"start_line":729,"start_character":23,"end_line":729,"end_character":38},"in_reply_to":"9fdfeff1_77aaf271","updated":"2019-02-21 09:37:42.000000000","message":"Done, in a fup","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a7d54d5a4e3e9d59e6faf7704b848cb0de0a04a3","unresolved":false,"context_lines":[{"line_number":737,"context_line":"                    if rp_allocs[rc] !\u003d 0:"},{"line_number":738,"context_line":"                        LOG.debug("},{"line_number":739,"context_line":"                            \u0027Found valid group - RP mapping %s but there are \u0027"},{"line_number":740,"context_line":"                            \u0027allocation leftover in %s\u0027,"},{"line_number":741,"context_line":"                            group_rp_mapping, allocs)"},{"line_number":742,"context_line":"                        return False"},{"line_number":743,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_e68f1a2c","line":740,"range":{"start_line":740,"start_character":29,"end_line":740,"end_character":48},"updated":"2019-02-20 15:00:15.000000000","message":"allocations left over\n\nand mentioning the RC could be helpful too.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":737,"context_line":"                    if rp_allocs[rc] !\u003d 0:"},{"line_number":738,"context_line":"                        LOG.debug("},{"line_number":739,"context_line":"                            \u0027Found valid group - RP mapping %s but there are \u0027"},{"line_number":740,"context_line":"                            \u0027allocation leftover in %s\u0027,"},{"line_number":741,"context_line":"                            group_rp_mapping, allocs)"},{"line_number":742,"context_line":"                        return False"},{"line_number":743,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_fb48ffb3","line":740,"range":{"start_line":740,"start_character":29,"end_line":740,"end_character":48},"in_reply_to":"9fdfeff1_e68f1a2c","updated":"2019-02-21 09:37:42.000000000","message":"Done in a fup","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a7d54d5a4e3e9d59e6faf7704b848cb0de0a04a3","unresolved":false,"context_lines":[{"line_number":738,"context_line":"                        LOG.debug("},{"line_number":739,"context_line":"                            \u0027Found valid group - RP mapping %s but there are \u0027"},{"line_number":740,"context_line":"                            \u0027allocation leftover in %s\u0027,"},{"line_number":741,"context_line":"                            group_rp_mapping, allocs)"},{"line_number":742,"context_line":"                        return False"},{"line_number":743,"context_line":""},{"line_number":744,"context_line":"        # If both the traits and the allocations are OK then mapping is valid"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_66924a4e","line":741,"range":{"start_line":741,"start_character":28,"end_line":741,"end_character":44},"updated":"2019-02-20 15:00:15.000000000","message":"is %s\u0027ing this going to produce anything informative? \u003cRequestGroup at 0xabcd1234\u003e ?","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":738,"context_line":"                        LOG.debug("},{"line_number":739,"context_line":"                            \u0027Found valid group - RP mapping %s but there are \u0027"},{"line_number":740,"context_line":"                            \u0027allocation leftover in %s\u0027,"},{"line_number":741,"context_line":"                            group_rp_mapping, allocs)"},{"line_number":742,"context_line":"                        return False"},{"line_number":743,"context_line":""},{"line_number":744,"context_line":"        # If both the traits and the allocations are OK then mapping is valid"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_1b75c3cc","line":741,"range":{"start_line":741,"start_character":28,"end_line":741,"end_character":44},"in_reply_to":"9fdfeff1_17b5a6cd","updated":"2019-02-21 09:37:42.000000000","message":"From the test log (I did a reformatting for readability):\n\n    2019-02-21 04:04:15,790 DEBUG [nova.objects.request_spec] Found valid group - RP mapping [\n\n(RequestGroup(aggregates\u003d[],forbidden_traits\u003dset([]),provider_uuids\u003d[],requester_id\u003dNone,required_traits\u003dset([\u0027CUSTOM_PHYSNET_PHYSNET0\u0027,\u0027CUSTOM_VNIC_TYPE_NORMAL\u0027]),resources\u003d{NET_BW_EGR_KILOBIT_PER_SEC\u003d1,NET_BW_IGR_KILOBIT_PER_SEC\u003d1},use_same_provider\u003dTrue), \n\u0027212e6d55-25eb-4e67-b1b5-e50780d218c1\u0027), \n\n(RequestGroup(aggregates\u003d[],forbidden_traits\u003dset([]),provider_uuids\u003d[],requester_id\u003dNone,required_traits\u003dset([\u0027CUSTOM_PHYSNET_PHYSNET0\u0027,\u0027CUSTOM_VNIC_TYPE_NORMAL\u0027]),resources\u003d{NET_BW_EGR_KILOBIT_PER_SEC\u003d1,NET_BW_IGR_KILOBIT_PER_SEC\u003d1},use_same_provider\u003dTrue), \u00275e19bdf5-6c75-4a6c-a9d0-c391a5960900\u0027)\n] \n\nbut there are allocations leftover in \n\nOrderedDict([(\u002733cf28b1-f3b9-45ef-a993-1364e7b9fc03\u0027, {\u0027resources\u0027: {\u0027VCPU\u0027: 1}}), (\u00275e19bdf5-6c75-4a6c-a9d0-c391a5960900\u0027, {\u0027resources\u0027: {\u0027NET_BW_IGR_KILOBIT_PER_SEC\u0027: 1, \u0027NET_BW_EGR_KILOBIT_PER_SEC\u0027: 1}}), (\u0027212e6d55-25eb-4e67-b1b5-e50780d218c1\u0027, {\u0027resources\u0027: {\u0027NET_BW_IGR_KILOBIT_PER_SEC\u0027: 0, \u0027NET_BW_EGR_KILOBIT_PER_SEC\u0027: 0}})]) \n\nfrom resource class NET_BW_IGR_KILOBIT_PER_SEC","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":738,"context_line":"                        LOG.debug("},{"line_number":739,"context_line":"                            \u0027Found valid group - RP mapping %s but there are \u0027"},{"line_number":740,"context_line":"                            \u0027allocation leftover in %s\u0027,"},{"line_number":741,"context_line":"                            group_rp_mapping, allocs)"},{"line_number":742,"context_line":"                        return False"},{"line_number":743,"context_line":""},{"line_number":744,"context_line":"        # If both the traits and the allocations are OK then mapping is valid"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_17b5a6cd","line":741,"range":{"start_line":741,"start_character":28,"end_line":741,"end_character":44},"in_reply_to":"9fdfeff1_66924a4e","updated":"2019-02-20 17:45:07.000000000","message":"All ovo\u0027s have a __str__ override so it will probably look something like {RequestGroup(id\u003d2,resources\u003ddict...)): [Allocation(resource_class\u003dDISK_GB, total\u003d2), }","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a7d54d5a4e3e9d59e6faf7704b848cb0de0a04a3","unresolved":false,"context_lines":[{"line_number":759,"context_line":"        * mapping is generated only based on the resource request and the"},{"line_number":760,"context_line":"          required traits, aggregate membership and forbidden traits are not"},{"line_number":761,"context_line":"          supported."},{"line_number":762,"context_line":"        * requesting the same resource class in numbered and un-numbered group"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"        We can live with these limitations today as Neutron does not use"},{"line_number":765,"context_line":"        forbidden traits and aggregates in the request and each Neutron port is"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_26b522ce","line":762,"updated":"2019-02-20 15:00:15.000000000","message":"is not supported","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":759,"context_line":"        * mapping is generated only based on the resource request and the"},{"line_number":760,"context_line":"          required traits, aggregate membership and forbidden traits are not"},{"line_number":761,"context_line":"          supported."},{"line_number":762,"context_line":"        * requesting the same resource class in numbered and un-numbered group"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"        We can live with these limitations today as Neutron does not use"},{"line_number":765,"context_line":"        forbidden traits and aggregates in the request and each Neutron port is"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_bb32770e","line":762,"in_reply_to":"9fdfeff1_26b522ce","updated":"2019-02-21 09:37:42.000000000","message":"Done in a fup","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        This is a workaround as placement does not return which RP fulfills"},{"line_number":770,"context_line":"        which granular request group in the allocation candidate request. There"},{"line_number":771,"context_line":"        is a spec proposing a solution in placement:"},{"line_number":772,"context_line":"        https://review.openstack.org/#/c/597601/"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        :param placement_allocations: The overall allocation made by the"},{"line_number":775,"context_line":"                                      scheduler for this RequestSpec"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_328d582a","line":772,"updated":"2019-02-20 17:45:07.000000000","message":"✔","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"899a82a6b047b3dbaef5d313af2e32c57b763748","unresolved":false,"context_lines":[{"line_number":718,"context_line":"            rp_allocs \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":719,"context_line":"            for rc, amount in group.resources.items():"},{"line_number":720,"context_line":"                rcs.add(rc)"},{"line_number":721,"context_line":"                if rc in rp_allocs:"},{"line_number":722,"context_line":"                    rp_allocs[rc] -\u003d amount"},{"line_number":723,"context_line":"                    if rp_allocs[rc] \u003c 0:"},{"line_number":724,"context_line":"                        return False"},{"line_number":725,"context_line":"                else:"},{"line_number":726,"context_line":"                    return False"},{"line_number":727,"context_line":""},{"line_number":728,"context_line":"        # Check that all the allocations are consumed from the resource"},{"line_number":729,"context_line":"        # classes that are appeared in the request groups. It should never"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fdfeff1_6260d636","line":726,"range":{"start_line":721,"start_character":16,"end_line":726,"end_character":32},"updated":"2019-02-21 20:04:17.000000000","message":"Comments here would be nice for each condition. First is the allocation drops below 0 so it can\u0027t be a candidate resource provider. Second is the provider does not provide the required resource class.","commit_id":"2fc904acf6d13fed8f81a9dbb88a1bfce06287da"}],"nova/tests/unit/objects/test_request_spec.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":988,"context_line":"                \u0027CUSTOM_VNIC_TYPE_NORMAL\u0027"},{"line_number":989,"context_line":"            ],"},{"line_number":990,"context_line":"        }"},{"line_number":991,"context_line":"        group1 \u003d request_spec.RequestGroup("},{"line_number":992,"context_line":"            resources\u003d{"},{"line_number":993,"context_line":"                \"NET_BW_IGR_KILOBIT_PER_SEC\": 1,"},{"line_number":994,"context_line":"                \"NET_BW_EGR_KILOBIT_PER_SEC\": 1,"},{"line_number":995,"context_line":"            },"},{"line_number":996,"context_line":"            required_traits\u003d{"},{"line_number":997,"context_line":"                \"CUSTOM_PHYSNET_PHYSNET0\","},{"line_number":998,"context_line":"                \"CUSTOM_VNIC_TYPE_NORMAL\","},{"line_number":999,"context_line":"            })"},{"line_number":1000,"context_line":"        self.spec.requested_resources \u003d [group1]"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"        self.spec.map_requested_resources_to_providers("},{"line_number":1003,"context_line":"            allocations, provider_traits)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_ebfc5f73","line":1000,"range":{"start_line":991,"start_character":8,"end_line":1000,"end_character":48},"updated":"2019-02-12 19:53:06.000000000","message":"But wait, if this was the only request group, how did the VCPU resource get allocated?","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":988,"context_line":"                \u0027CUSTOM_VNIC_TYPE_NORMAL\u0027"},{"line_number":989,"context_line":"            ],"},{"line_number":990,"context_line":"        }"},{"line_number":991,"context_line":"        group1 \u003d request_spec.RequestGroup("},{"line_number":992,"context_line":"            resources\u003d{"},{"line_number":993,"context_line":"                \"NET_BW_IGR_KILOBIT_PER_SEC\": 1,"},{"line_number":994,"context_line":"                \"NET_BW_EGR_KILOBIT_PER_SEC\": 1,"},{"line_number":995,"context_line":"            },"},{"line_number":996,"context_line":"            required_traits\u003d{"},{"line_number":997,"context_line":"                \"CUSTOM_PHYSNET_PHYSNET0\","},{"line_number":998,"context_line":"                \"CUSTOM_VNIC_TYPE_NORMAL\","},{"line_number":999,"context_line":"            })"},{"line_number":1000,"context_line":"        self.spec.requested_resources \u003d [group1]"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"        self.spec.map_requested_resources_to_providers("},{"line_number":1003,"context_line":"            allocations, provider_traits)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_8b97a545","line":1000,"range":{"start_line":991,"start_character":8,"end_line":1000,"end_character":48},"in_reply_to":"9fdfeff1_ebfc5f73","updated":"2019-02-14 14:06:16.000000000","message":"This was the only RequestGroup in the RequestSpec. But when the scheduler generates the resource request [1] it also considers other part of the RequestSpec. \n\nI agree that next independent step could be to refactor nova to put the unnumbered group as a RequestGroup into the RequestSpec and then [1] can be simplified a lot. Still for the unnumbered group we need special handling in the mapping generation as that code cannot handle unnumbered group yet. (and I hope it will never need to handle it as the proposed placement a_c improvement will make this hole code obsolete.)\n\n\n[1] https://github.com/openstack/nova/blob/fae8b3efcde73fde417c1db6237451fd0d1545f0/nova/scheduler/utils.py#L412","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":1133,"context_line":"        self.assertEqual([uuids.net_dev1_rp], group2.provider_uuids)"},{"line_number":1134,"context_line":""},{"line_number":1135,"context_line":"    def test_two_groups_different_providers(self):"},{"line_number":1136,"context_line":"        # NOTE(gibi): we use OrderedDict here to make the test deterministic"},{"line_number":1137,"context_line":"        allocations \u003d collections.OrderedDict()"},{"line_number":1138,"context_line":"        allocations[uuids.compute1_rp] \u003d {"},{"line_number":1139,"context_line":"            \"resources\": {"},{"line_number":1140,"context_line":"                    \u0027VCPU\u0027: 1"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_eb377ffd","line":1137,"range":{"start_line":1136,"start_character":0,"end_line":1137,"end_character":47},"updated":"2019-02-12 19:53:06.000000000","message":"oh, look what you did :)\n\nNote to self: This works, despite uuidsentinel being nondeterministic, because OrderedDict orders *chronologically*, not numerically/lexicographically/otherwise by key.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"525115fa9b908cfa9548c3ad1d6c17a78c958600","unresolved":false,"context_lines":[{"line_number":1133,"context_line":"        self.assertEqual([uuids.net_dev1_rp], group2.provider_uuids)"},{"line_number":1134,"context_line":""},{"line_number":1135,"context_line":"    def test_two_groups_different_providers(self):"},{"line_number":1136,"context_line":"        # NOTE(gibi): we use OrderedDict here to make the test deterministic"},{"line_number":1137,"context_line":"        allocations \u003d collections.OrderedDict()"},{"line_number":1138,"context_line":"        allocations[uuids.compute1_rp] \u003d {"},{"line_number":1139,"context_line":"            \"resources\": {"},{"line_number":1140,"context_line":"                    \u0027VCPU\u0027: 1"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_23eb3d78","line":1137,"range":{"start_line":1136,"start_character":0,"end_line":1137,"end_character":47},"in_reply_to":"9fdfeff1_4b653db4","updated":"2019-02-14 18:08:06.000000000","message":"Yes, it\u0027s very clever. I\u0027m sure I would have gone off in the weeds implementing the ordering on the code side.","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"78652a63908b435d9d8caadcbaa74df01eea6de7","unresolved":false,"context_lines":[{"line_number":1133,"context_line":"        self.assertEqual([uuids.net_dev1_rp], group2.provider_uuids)"},{"line_number":1134,"context_line":""},{"line_number":1135,"context_line":"    def test_two_groups_different_providers(self):"},{"line_number":1136,"context_line":"        # NOTE(gibi): we use OrderedDict here to make the test deterministic"},{"line_number":1137,"context_line":"        allocations \u003d collections.OrderedDict()"},{"line_number":1138,"context_line":"        allocations[uuids.compute1_rp] \u003d {"},{"line_number":1139,"context_line":"            \"resources\": {"},{"line_number":1140,"context_line":"                    \u0027VCPU\u0027: 1"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_4b653db4","line":1137,"range":{"start_line":1136,"start_character":0,"end_line":1137,"end_character":47},"in_reply_to":"9fdfeff1_eb377ffd","updated":"2019-02-14 14:06:16.000000000","message":"If only the test needs deterministic ordering the lets only use deterministic ordering only in the test. :)","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e8c7ddf1d0c929e6ae33d272c806f7cd605e9ecc","unresolved":false,"context_lines":[{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        # the algorithm tried out many possible mappings before found the"},{"line_number":1390,"context_line":"        # the solution"},{"line_number":1391,"context_line":"        self.assertEqual(58, mock_validator.call_count)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fdfeff1_462ec292","line":1391,"range":{"start_line":1391,"start_character":25,"end_line":1391,"end_character":27},"updated":"2019-02-12 19:53:06.000000000","message":"noyce","commit_id":"f2ba72e88cab86b2ab72aace9fd96c77fcb1d012"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":1008,"context_line":"        # NOTE(gibi): This negative test scenario should not happen in real"},{"line_number":1009,"context_line":"        # end to end test as we assume that placement only returns candidates"},{"line_number":1010,"context_line":"        # that are valid. But still we want to cover the error case in our"},{"line_number":1011,"context_line":"        # implementation"},{"line_number":1012,"context_line":""},{"line_number":1013,"context_line":"        allocations \u003d {"},{"line_number":1014,"context_line":"            uuids.compute1_rp: {"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_d2c64c4a","line":1011,"updated":"2019-02-20 17:45:07.000000000","message":"✔","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":1342,"context_line":"                \u0027CUSTOM_PHYSNET_PHYSNET0\u0027, \u0027CUSTOM_VNIC_TYPE_NORMAL\u0027"},{"line_number":1343,"context_line":"            ],"},{"line_number":1344,"context_line":"        }"},{"line_number":1345,"context_line":"        # this fits to 3 RPs"},{"line_number":1346,"context_line":"        group1 \u003d request_spec.RequestGroup("},{"line_number":1347,"context_line":"            resources\u003d{"},{"line_number":1348,"context_line":"                \"NET_BW_IGR_KILOBIT_PER_SEC\": 1,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_522edcfa","line":1345,"updated":"2019-02-20 17:45:07.000000000","message":"technically, no, this only fits to rp1 and rp3. rp2 has a egress inventory of 2 which is less than the requested amount of 3.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":1342,"context_line":"                \u0027CUSTOM_PHYSNET_PHYSNET0\u0027, \u0027CUSTOM_VNIC_TYPE_NORMAL\u0027"},{"line_number":1343,"context_line":"            ],"},{"line_number":1344,"context_line":"        }"},{"line_number":1345,"context_line":"        # this fits to 3 RPs"},{"line_number":1346,"context_line":"        group1 \u003d request_spec.RequestGroup("},{"line_number":1347,"context_line":"            resources\u003d{"},{"line_number":1348,"context_line":"                \"NET_BW_IGR_KILOBIT_PER_SEC\": 1,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_9b18d384","line":1345,"in_reply_to":"9fdfeff1_522edcfa","updated":"2019-02-21 09:37:42.000000000","message":"Good point. I needed to trim this example in the last spin but missed the comments. Fixed the comment in a fup.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a7d54d5a4e3e9d59e6faf7704b848cb0de0a04a3","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"        # NOTE(gibi): This negative test scenario should not happen in real"},{"line_number":1396,"context_line":"        # end to end test as we assume that placement only returns candidates"},{"line_number":1397,"context_line":"        # that are valid and this candidate is not valid as it provides more"},{"line_number":1398,"context_line":"        # resources that the ports are requesting. Still we want to cover the"},{"line_number":1399,"context_line":"        # error case in our implementation"},{"line_number":1400,"context_line":""},{"line_number":1401,"context_line":"        allocations \u003d collections.OrderedDict()"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_c6f7768c","line":1398,"range":{"start_line":1398,"start_character":20,"end_line":1398,"end_character":24},"updated":"2019-02-20 15:00:15.000000000","message":"than","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"        # NOTE(gibi): This negative test scenario should not happen in real"},{"line_number":1396,"context_line":"        # end to end test as we assume that placement only returns candidates"},{"line_number":1397,"context_line":"        # that are valid and this candidate is not valid as it provides more"},{"line_number":1398,"context_line":"        # resources that the ports are requesting. Still we want to cover the"},{"line_number":1399,"context_line":"        # error case in our implementation"},{"line_number":1400,"context_line":""},{"line_number":1401,"context_line":"        allocations \u003d collections.OrderedDict()"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_b6004e02","line":1398,"range":{"start_line":1398,"start_character":20,"end_line":1398,"end_character":24},"in_reply_to":"9fdfeff1_c6f7768c","updated":"2019-02-21 09:37:42.000000000","message":"Done in a fup.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a7d54d5a4e3e9d59e6faf7704b848cb0de0a04a3","unresolved":false,"context_lines":[{"line_number":1401,"context_line":"        allocations \u003d collections.OrderedDict()"},{"line_number":1402,"context_line":"        allocations[uuids.compute1_rp] \u003d {"},{"line_number":1403,"context_line":"            \"resources\": {"},{"line_number":1404,"context_line":"                \u0027VCPU\u0027: 1"},{"line_number":1405,"context_line":"            }"},{"line_number":1406,"context_line":"        }"},{"line_number":1407,"context_line":"        allocations[uuids.net_dev1_physnet0_rp] \u003d {"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_662ecafb","line":1404,"updated":"2019-02-20 15:00:15.000000000","message":"A clearer simulation of the problem would have been if this (the compute) RP contained NET_BW_*.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"59945a2c82f162e0a9adc3c347d0bcec12f2f08d","unresolved":false,"context_lines":[{"line_number":1401,"context_line":"        allocations \u003d collections.OrderedDict()"},{"line_number":1402,"context_line":"        allocations[uuids.compute1_rp] \u003d {"},{"line_number":1403,"context_line":"            \"resources\": {"},{"line_number":1404,"context_line":"                \u0027VCPU\u0027: 1"},{"line_number":1405,"context_line":"            }"},{"line_number":1406,"context_line":"        }"},{"line_number":1407,"context_line":"        allocations[uuids.net_dev1_physnet0_rp] \u003d {"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_760ae620","line":1404,"in_reply_to":"9fdfeff1_5217bca4","updated":"2019-02-21 09:37:42.000000000","message":"Yeah. I agree with Jay here.","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3326c2c2381069fdbdd25fa1e5897380449188d","unresolved":false,"context_lines":[{"line_number":1401,"context_line":"        allocations \u003d collections.OrderedDict()"},{"line_number":1402,"context_line":"        allocations[uuids.compute1_rp] \u003d {"},{"line_number":1403,"context_line":"            \"resources\": {"},{"line_number":1404,"context_line":"                \u0027VCPU\u0027: 1"},{"line_number":1405,"context_line":"            }"},{"line_number":1406,"context_line":"        }"},{"line_number":1407,"context_line":"        allocations[uuids.net_dev1_physnet0_rp] \u003d {"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fdfeff1_5217bca4","line":1404,"in_reply_to":"9fdfeff1_662ecafb","updated":"2019-02-20 17:45:07.000000000","message":"but the compute node isn\u0027t really the provider that has NET_BW_* inventory :)","commit_id":"5a8b0956f6a5e9ca2be4388129de7ca9728cc27b"}]}