)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ebfcf396fbdded83f26e922e55d1a516975290d5","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Return nested providers in get_by_request"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch start to return nested providers in get_by_requests()."},{"line_number":10,"context_line":"However, this is not complete because"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"1) It leaves a bug that placement returns a \"branch\" without requested"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"5f7c97a3_1b9dd158","line":9,"range":{"start_line":9,"start_character":11,"end_line":9,"end_character":16},"updated":"2018-05-18 20:20:37.000000000","message":"starts","commit_id":"ce4892309fbbeb84d0e506cfaa044be7e4755d56"}],"nova/api/openstack/placement/objects/resource_provider.py":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"2ce8a73e6973edf64ee068baad5fe8b429655be6","unresolved":false,"context_lines":[{"line_number":3318,"context_line":""},{"line_number":3319,"context_line":"    outer_rps \u003d sa.alias(_RP_TBL, name\u003d\"outer_rps\")"},{"line_number":3320,"context_line":"    outer_to_subq \u003d sa.join("},{"line_number":3321,"context_line":"        outer_rps, trees_with_traits,"},{"line_number":3322,"context_line":"        outer_rps.c.root_provider_id \u003d\u003d trees_with_traits.c.root_provider_id)"},{"line_number":3323,"context_line":"    sel \u003d sa.select([outer_rps.c.id, outer_rps.c.root_provider_id])"},{"line_number":3324,"context_line":"    sel \u003d sel.select_from(outer_to_subq)"},{"line_number":3325,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_5515289b","line":3322,"range":{"start_line":3321,"start_character":0,"end_line":3322,"end_character":77},"updated":"2018-05-09 06:46:45.000000000","message":"Note: \n\nAs I wrote in the commit message,  there is a trait related bug. This is where trees without required traits are excluded. But the problem is that the accuracy is not by rps but by trees, so we can\u0027t avoid checking it again in details later in _alloc_candidates_no_shared().\n\nIf no objection, I will submit that patch shortly since I have no idea to fix it within this function in SQL...","commit_id":"f8b204248ffe89b70d81c8cbd723073915f53346"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b952a360d0c8e3e0cacd9a5523434f1f382f59f9","unresolved":false,"context_lines":[{"line_number":3597,"context_line":"        for res_requests in itertools.product(*request_groups):"},{"line_number":3598,"context_line":"            alloc_requests.append("},{"line_number":3599,"context_line":"                AllocationRequest(ctx, resource_requests\u003dlist(res_requests),"},{"line_number":3600,"context_line":"                                  anchor_root_provider_uuid\u003droot_id)"},{"line_number":3601,"context_line":"            )"},{"line_number":3602,"context_line":"    return alloc_requests, list(summaries.values())"},{"line_number":3603,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_3490a543","line":3600,"range":{"start_line":3600,"start_character":60,"end_line":3600,"end_character":67},"updated":"2018-05-09 13:53:25.000000000","message":"Ah, uuid!","commit_id":"d7fc781a5595504d3471a4e54a6e855db66c31b6"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3127,"context_line":"    :param ctx: Session context to use"},{"line_number":3128,"context_line":"    :param rc_id: Internal ID of resource class to check inventory for"},{"line_number":3129,"context_line":"    :param amount: Amount of resource being requested"},{"line_number":3130,"context_line":"    :param member_of: An optional list of aggregate UUIDs. If provided,"},{"line_number":3131,"context_line":"                      the allocation_candidates returned will only be for"},{"line_number":3132,"context_line":"                      resource providers that are members of one or more"},{"line_number":3133,"context_line":"                      of the supplied aggregates."}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_c76abceb","line":3130,"range":{"start_line":3130,"start_character":34,"end_line":3130,"end_character":42},"updated":"2018-05-10 22:24:18.000000000","message":"list of lists of","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3142,"context_line":"    #    alloc.resource_provider_id,"},{"line_number":3143,"context_line":"    #    SUM(allocs.used) AS used"},{"line_number":3144,"context_line":"    #  FROM allocations AS alloc"},{"line_number":3145,"context_line":"    #  WHERE allocs.resource_class_id \u003d $RC_ID"},{"line_number":3146,"context_line":"    #  GROUP BY allocs.resource_provider_id"},{"line_number":3147,"context_line":"    # ) AS usage"},{"line_number":3148,"context_line":"    #  ON inv.resource_provider_id \u003d usage.resource_provider_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_a72300bd","line":3145,"updated":"2018-05-10 22:24:18.000000000","message":"this needs to be updated (if you\u0027re keeping the aggregate business)","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3174,"context_line":"    inv_to_usage \u003d sa.outerjoin("},{"line_number":3175,"context_line":"        rp_to_inv, usage,"},{"line_number":3176,"context_line":"        inv.c.resource_provider_id \u003d\u003d usage.c.resource_provider_id)"},{"line_number":3177,"context_line":"    sel \u003d sa.select([rpt.c.id, rpt.c.root_provider_id])"},{"line_number":3178,"context_line":"    sel \u003d sel.select_from(inv_to_usage)"},{"line_number":3179,"context_line":"    if member_of:"},{"line_number":3180,"context_line":"        rps_in_aggs \u003d _provider_ids_matching_aggregates(ctx, member_of)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_272130b1","line":3177,"range":{"start_line":3177,"start_character":31,"end_line":3177,"end_character":53},"updated":"2018-05-10 22:24:18.000000000","message":"Don\u0027t we need to account for this being null?","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3c824f83c16aaeff91b7949d3a0fd4311031f136","unresolved":false,"context_lines":[{"line_number":3174,"context_line":"    inv_to_usage \u003d sa.outerjoin("},{"line_number":3175,"context_line":"        rp_to_inv, usage,"},{"line_number":3176,"context_line":"        inv.c.resource_provider_id \u003d\u003d usage.c.resource_provider_id)"},{"line_number":3177,"context_line":"    sel \u003d sa.select([rpt.c.id, rpt.c.root_provider_id])"},{"line_number":3178,"context_line":"    sel \u003d sel.select_from(inv_to_usage)"},{"line_number":3179,"context_line":"    if member_of:"},{"line_number":3180,"context_line":"        rps_in_aggs \u003d _provider_ids_matching_aggregates(ctx, member_of)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_7cefa41d","line":3177,"range":{"start_line":3177,"start_character":31,"end_line":3177,"end_character":53},"in_reply_to":"5f7c97a3_0d1c5376","updated":"2018-05-12 07:23:11.000000000","message":"(Later) Seems that this is okay (while we should care joining tables on this parameter). See test_root_provider_population(self) at [1]. \n\n[1] https://github.com/openstack/nova/blob/master/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py#L105.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"405fda7c3e74e4ff11af1a960f463a2f162df53f","unresolved":false,"context_lines":[{"line_number":3174,"context_line":"    inv_to_usage \u003d sa.outerjoin("},{"line_number":3175,"context_line":"        rp_to_inv, usage,"},{"line_number":3176,"context_line":"        inv.c.resource_provider_id \u003d\u003d usage.c.resource_provider_id)"},{"line_number":3177,"context_line":"    sel \u003d sa.select([rpt.c.id, rpt.c.root_provider_id])"},{"line_number":3178,"context_line":"    sel \u003d sel.select_from(inv_to_usage)"},{"line_number":3179,"context_line":"    if member_of:"},{"line_number":3180,"context_line":"        rps_in_aggs \u003d _provider_ids_matching_aggregates(ctx, member_of)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_0d1c5376","line":3177,"range":{"start_line":3177,"start_character":31,"end_line":3177,"end_character":53},"in_reply_to":"5f7c97a3_272130b1","updated":"2018-05-11 04:43:52.000000000","message":"Good point... We should work around that case. Noted on the top of the function. I\u0027ll add test cases for this later.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3176,"context_line":"        inv.c.resource_provider_id \u003d\u003d usage.c.resource_provider_id)"},{"line_number":3177,"context_line":"    sel \u003d sa.select([rpt.c.id, rpt.c.root_provider_id])"},{"line_number":3178,"context_line":"    sel \u003d sel.select_from(inv_to_usage)"},{"line_number":3179,"context_line":"    if member_of:"},{"line_number":3180,"context_line":"        rps_in_aggs \u003d _provider_ids_matching_aggregates(ctx, member_of)"},{"line_number":3181,"context_line":"        if not rps_in_aggs:"},{"line_number":3182,"context_line":"            # Short-circuit. The user either asked for a non-existing"},{"line_number":3183,"context_line":"            # aggregate or there were no resource providers that matched"},{"line_number":3184,"context_line":"            # the requirements..."},{"line_number":3185,"context_line":"            return []"},{"line_number":3186,"context_line":"        where_conds.append(rpt.c.root_provider_id.in_(rps_in_aggs))"},{"line_number":3187,"context_line":"    sel \u003d sel.where(sa.and_(*where_conds))"},{"line_number":3188,"context_line":"    res \u003d ctx.session.execute(sel).fetchall()"},{"line_number":3189,"context_line":"    res \u003d set((r[0], r[1], rc_id) for r in res)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_67a64856","line":3186,"range":{"start_line":3179,"start_character":0,"end_line":3186,"end_character":67},"updated":"2018-05-10 22:24:18.000000000","message":"If you\u0027re not going to do this via a JOIN, it may not make sense to do it within this method at all.  Or rename the method _get_providers_with_resource_in_aggregates.  But then what about traits?","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"405fda7c3e74e4ff11af1a960f463a2f162df53f","unresolved":false,"context_lines":[{"line_number":3176,"context_line":"        inv.c.resource_provider_id \u003d\u003d usage.c.resource_provider_id)"},{"line_number":3177,"context_line":"    sel \u003d sa.select([rpt.c.id, rpt.c.root_provider_id])"},{"line_number":3178,"context_line":"    sel \u003d sel.select_from(inv_to_usage)"},{"line_number":3179,"context_line":"    if member_of:"},{"line_number":3180,"context_line":"        rps_in_aggs \u003d _provider_ids_matching_aggregates(ctx, member_of)"},{"line_number":3181,"context_line":"        if not rps_in_aggs:"},{"line_number":3182,"context_line":"            # Short-circuit. The user either asked for a non-existing"},{"line_number":3183,"context_line":"            # aggregate or there were no resource providers that matched"},{"line_number":3184,"context_line":"            # the requirements..."},{"line_number":3185,"context_line":"            return []"},{"line_number":3186,"context_line":"        where_conds.append(rpt.c.root_provider_id.in_(rps_in_aggs))"},{"line_number":3187,"context_line":"    sel \u003d sel.where(sa.and_(*where_conds))"},{"line_number":3188,"context_line":"    res \u003d ctx.session.execute(sel).fetchall()"},{"line_number":3189,"context_line":"    res \u003d set((r[0], r[1], rc_id) for r in res)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_8d0f0336","line":3186,"range":{"start_line":3179,"start_character":0,"end_line":3186,"end_character":67},"in_reply_to":"5f7c97a3_67a64856","updated":"2018-05-11 04:43:52.000000000","message":"Put this out of this function.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3186,"context_line":"        where_conds.append(rpt.c.root_provider_id.in_(rps_in_aggs))"},{"line_number":3187,"context_line":"    sel \u003d sel.where(sa.and_(*where_conds))"},{"line_number":3188,"context_line":"    res \u003d ctx.session.execute(sel).fetchall()"},{"line_number":3189,"context_line":"    res \u003d set((r[0], r[1], rc_id) for r in res)"},{"line_number":3190,"context_line":"    return res"},{"line_number":3191,"context_line":""},{"line_number":3192,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_0765141e","line":3189,"range":{"start_line":3189,"start_character":25,"end_line":3189,"end_character":32},"updated":"2018-05-10 22:24:18.000000000","message":"This is weird, since it\u0027ll be the same for every record, and the caller already knows it.  I guess I\u0027ll see how you\u0027re using this later on...","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"405fda7c3e74e4ff11af1a960f463a2f162df53f","unresolved":false,"context_lines":[{"line_number":3186,"context_line":"        where_conds.append(rpt.c.root_provider_id.in_(rps_in_aggs))"},{"line_number":3187,"context_line":"    sel \u003d sel.where(sa.and_(*where_conds))"},{"line_number":3188,"context_line":"    res \u003d ctx.session.execute(sel).fetchall()"},{"line_number":3189,"context_line":"    res \u003d set((r[0], r[1], rc_id) for r in res)"},{"line_number":3190,"context_line":"    return res"},{"line_number":3191,"context_line":""},{"line_number":3192,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_4d056b13","line":3189,"range":{"start_line":3189,"start_character":25,"end_line":3189,"end_character":32},"in_reply_to":"5f7c97a3_0765141e","updated":"2018-05-11 04:43:52.000000000","message":"Put this out of this function.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3544,"context_line":""},{"line_number":3545,"context_line":""},{"line_number":3546,"context_line":"def _alloc_candidates_no_shared(ctx, requested_resources, rp_tuples):"},{"line_number":3547,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3548,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3549,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"},{"line_number":3550,"context_line":"    in the requested resources as well as ALL required traits that were"},{"line_number":3551,"context_line":"    requested by the user."},{"line_number":3552,"context_line":""},{"line_number":3553,"context_line":"    This is an optimized code path for the common scenario when no sharing"},{"line_number":3554,"context_line":"    providers exist in the system for any requested resource. In this scenario,"},{"line_number":3555,"context_line":"    we can more efficiently build the list of AllocationRequest and"},{"line_number":3556,"context_line":"    ProviderSummary objects due to not having to determine requests for some"},{"line_number":3557,"context_line":"    shared and some non-shared resources."},{"line_number":3558,"context_line":""},{"line_number":3559,"context_line":"    :param ctx: nova.context.RequestContext object"},{"line_number":3560,"context_line":"    :param requested_resources: dict, keyed by resource class ID, of amounts"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_c26dcacc","line":3557,"range":{"start_line":3547,"start_character":0,"end_line":3557,"end_character":41},"updated":"2018-05-10 22:24:18.000000000","message":"It\u0027s not clear from the method name or this docstring that this method is dealing with *trees*.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3c824f83c16aaeff91b7949d3a0fd4311031f136","unresolved":false,"context_lines":[{"line_number":3544,"context_line":""},{"line_number":3545,"context_line":""},{"line_number":3546,"context_line":"def _alloc_candidates_no_shared(ctx, requested_resources, rp_tuples):"},{"line_number":3547,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3548,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3549,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"},{"line_number":3550,"context_line":"    in the requested resources as well as ALL required traits that were"},{"line_number":3551,"context_line":"    requested by the user."},{"line_number":3552,"context_line":""},{"line_number":3553,"context_line":"    This is an optimized code path for the common scenario when no sharing"},{"line_number":3554,"context_line":"    providers exist in the system for any requested resource. In this scenario,"},{"line_number":3555,"context_line":"    we can more efficiently build the list of AllocationRequest and"},{"line_number":3556,"context_line":"    ProviderSummary objects due to not having to determine requests for some"},{"line_number":3557,"context_line":"    shared and some non-shared resources."},{"line_number":3558,"context_line":""},{"line_number":3559,"context_line":"    :param ctx: nova.context.RequestContext object"},{"line_number":3560,"context_line":"    :param requested_resources: dict, keyed by resource class ID, of amounts"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_06d97ff1","line":3557,"range":{"start_line":3547,"start_character":0,"end_line":3557,"end_character":41},"in_reply_to":"5f7c97a3_6d294799","updated":"2018-05-12 07:23:11.000000000","message":"Put it back now according to the discssion in L3609","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"405fda7c3e74e4ff11af1a960f463a2f162df53f","unresolved":false,"context_lines":[{"line_number":3544,"context_line":""},{"line_number":3545,"context_line":""},{"line_number":3546,"context_line":"def _alloc_candidates_no_shared(ctx, requested_resources, rp_tuples):"},{"line_number":3547,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3548,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3549,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"},{"line_number":3550,"context_line":"    in the requested resources as well as ALL required traits that were"},{"line_number":3551,"context_line":"    requested by the user."},{"line_number":3552,"context_line":""},{"line_number":3553,"context_line":"    This is an optimized code path for the common scenario when no sharing"},{"line_number":3554,"context_line":"    providers exist in the system for any requested resource. In this scenario,"},{"line_number":3555,"context_line":"    we can more efficiently build the list of AllocationRequest and"},{"line_number":3556,"context_line":"    ProviderSummary objects due to not having to determine requests for some"},{"line_number":3557,"context_line":"    shared and some non-shared resources."},{"line_number":3558,"context_line":""},{"line_number":3559,"context_line":"    :param ctx: nova.context.RequestContext object"},{"line_number":3560,"context_line":"    :param requested_resources: dict, keyed by resource class ID, of amounts"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_6d294799","line":3557,"range":{"start_line":3547,"start_character":0,"end_line":3557,"end_character":41},"in_reply_to":"5f7c97a3_c26dcacc","updated":"2018-05-11 04:43:52.000000000","message":"Tried to be clear with my poor English. Please let me know if it is unclear.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3569,"context_line":"    requested_rc_ids \u003d list(requested_resources)"},{"line_number":3570,"context_line":""},{"line_number":3571,"context_line":"    # Get all rp ids from rp_tuples including root rp ids."},{"line_number":3572,"context_line":"    rp_ids \u003d set(p[0] for p in rp_tuples) | set(p[1] for p in rp_tuples)"},{"line_number":3573,"context_line":""},{"line_number":3574,"context_line":"    usages \u003d _get_usages_by_provider_and_rc(ctx, rp_ids, requested_rc_ids)"},{"line_number":3575,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_625e3613","line":3572,"range":{"start_line":3572,"start_character":44,"end_line":3572,"end_character":71},"updated":"2018-05-10 22:24:18.000000000","message":"Why do you want usages (L3574) and traits (L3578) for root providers that aren\u0027t providing resource?","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"405fda7c3e74e4ff11af1a960f463a2f162df53f","unresolved":false,"context_lines":[{"line_number":3569,"context_line":"    requested_rc_ids \u003d list(requested_resources)"},{"line_number":3570,"context_line":""},{"line_number":3571,"context_line":"    # Get all rp ids from rp_tuples including root rp ids."},{"line_number":3572,"context_line":"    rp_ids \u003d set(p[0] for p in rp_tuples) | set(p[1] for p in rp_tuples)"},{"line_number":3573,"context_line":""},{"line_number":3574,"context_line":"    usages \u003d _get_usages_by_provider_and_rc(ctx, rp_ids, requested_rc_ids)"},{"line_number":3575,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_ed3d57dd","line":3572,"range":{"start_line":3572,"start_character":44,"end_line":3572,"end_character":71},"in_reply_to":"5f7c97a3_625e3613","updated":"2018-05-11 04:43:52.000000000","message":"I laid the foundations for future development of \"return all rps in a tree\".  But so far not needed. Deleted.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3590,"context_line":"        rp_uuid \u003d rp_summary.resource_provider.uuid"},{"line_number":3591,"context_line":"        tree_dict[root_id][rc_id].append("},{"line_number":3592,"context_line":"            AllocationRequestResource("},{"line_number":3593,"context_line":"                ctx, resource_provider\u003dResourceProvider(ctx,"},{"line_number":3594,"context_line":"                                                        uuid\u003drp_uuid),"},{"line_number":3595,"context_line":"                resource_class\u003d_RC_CACHE.string_from_id(rc_id),"},{"line_number":3596,"context_line":"                amount\u003drequested_resources[rc_id]))"},{"line_number":3597,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_82759266","line":3594,"range":{"start_line":3593,"start_character":39,"end_line":3594,"end_character":69},"updated":"2018-05-10 22:24:18.000000000","message":"Use ResourceProvider.get_by_uuid(...) here so you have access to the root_provider_uuid below.\n\nIn fact, if you do this up front, you could make the outer dict keyed by root UUID instead of internal ID, and that would save you some pain below.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"25957978aec374b5e3ecf3ecb74f80bc004a2cf7","unresolved":false,"context_lines":[{"line_number":3590,"context_line":"        rp_uuid \u003d rp_summary.resource_provider.uuid"},{"line_number":3591,"context_line":"        tree_dict[root_id][rc_id].append("},{"line_number":3592,"context_line":"            AllocationRequestResource("},{"line_number":3593,"context_line":"                ctx, resource_provider\u003dResourceProvider(ctx,"},{"line_number":3594,"context_line":"                                                        uuid\u003drp_uuid),"},{"line_number":3595,"context_line":"                resource_class\u003d_RC_CACHE.string_from_id(rc_id),"},{"line_number":3596,"context_line":"                amount\u003drequested_resources[rc_id]))"},{"line_number":3597,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_cdb6318b","line":3594,"range":{"start_line":3593,"start_character":39,"end_line":3594,"end_character":69},"in_reply_to":"5f7c97a3_82759266","updated":"2018-05-14 12:13:42.000000000","message":"(later)  Yes, that\u0027s absolutely better because we can retrieve more information from here! Done in PS10.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8b2036797aea7df2be4081dbd4c4aa853b7aa8c1","unresolved":false,"context_lines":[{"line_number":3606,"context_line":"        # AllocationRequestResource per requested resource class."},{"line_number":3607,"context_line":"        request_groups \u003d alloc_dict.values()"},{"line_number":3608,"context_line":""},{"line_number":3609,"context_line":"        root_summary \u003d summaries[root_id]"},{"line_number":3610,"context_line":"        root_uuid \u003d root_summary.resource_provider.uuid"},{"line_number":3611,"context_line":""},{"line_number":3612,"context_line":"        # Using itertools.product, we get all the combinations of resource"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_e2432612","line":3609,"range":{"start_line":3609,"start_character":8,"end_line":3609,"end_character":41},"updated":"2018-05-10 22:24:18.000000000","message":"[cont\u0027d from L3572] ...if it\u0027s only so you can get this, you shouldn\u0027t need it.  You should be able to get the root_uuid from the AllocationRequestResource.resource_provider.root_provider_uuid (if you use get_by_uuid as noted above).","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"88a2d17de01fa6d77524343851733800ab17a6b5","unresolved":false,"context_lines":[{"line_number":3606,"context_line":"        # AllocationRequestResource per requested resource class."},{"line_number":3607,"context_line":"        request_groups \u003d alloc_dict.values()"},{"line_number":3608,"context_line":""},{"line_number":3609,"context_line":"        root_summary \u003d summaries[root_id]"},{"line_number":3610,"context_line":"        root_uuid \u003d root_summary.resource_provider.uuid"},{"line_number":3611,"context_line":""},{"line_number":3612,"context_line":"        # Using itertools.product, we get all the combinations of resource"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_0cbaf508","line":3609,"range":{"start_line":3609,"start_character":8,"end_line":3609,"end_character":41},"in_reply_to":"5f7c97a3_063dff29","updated":"2018-05-14 01:38:58.000000000","message":"Added a test case within this patch \"for a request where the root provider isn\u0027t actually providing resource\".","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b2de541e411ca94dd0da003650c73499a3ffb0a","unresolved":false,"context_lines":[{"line_number":3606,"context_line":"        # AllocationRequestResource per requested resource class."},{"line_number":3607,"context_line":"        request_groups \u003d alloc_dict.values()"},{"line_number":3608,"context_line":""},{"line_number":3609,"context_line":"        root_summary \u003d summaries[root_id]"},{"line_number":3610,"context_line":"        root_uuid \u003d root_summary.resource_provider.uuid"},{"line_number":3611,"context_line":""},{"line_number":3612,"context_line":"        # Using itertools.product, we get all the combinations of resource"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_89a7edbd","line":3609,"range":{"start_line":3609,"start_character":8,"end_line":3609,"end_character":41},"in_reply_to":"5f7c97a3_886b71b5","updated":"2018-05-11 13:25:22.000000000","message":"Well look, if you leave this in place, then you can\u0027t remove the root IDs from L3572.  For a request where the root provider isn\u0027t actually providing resource, summaries[root_id] won\u0027t exist otherwise.  Which is why I suggested using get_by_uuid on L3593 - then you can get AllocationRequestResource.resource_provider.root_provider_uuid and you don\u0027t *need* to include the root_ids on L3572.\n\nThe root_id with which you are indexing tree_dict is the root of the provider associated with AllocationRequestResource, as opposed to the \"anchor\".  When we\u0027re *not* dealing with sharing providers, those are the same thing; when we *are* dealing with sharing providers, we will have to be very careful to make sure we\u0027re using the right one.  And we have to account for cases where there use_same_provider\u003dFalse group only provides resources from sharing providers.  (This is what I ran into in the granular patch, and was the reason for introducing _anchors_for_sharing_provider.)\n\nI think what I\u0027m getting at is that this method needs to deal wholeheartedly with either trees-without-sharing or trees-with-sharing.  It can\u0027t go part way.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3c824f83c16aaeff91b7949d3a0fd4311031f136","unresolved":false,"context_lines":[{"line_number":3606,"context_line":"        # AllocationRequestResource per requested resource class."},{"line_number":3607,"context_line":"        request_groups \u003d alloc_dict.values()"},{"line_number":3608,"context_line":""},{"line_number":3609,"context_line":"        root_summary \u003d summaries[root_id]"},{"line_number":3610,"context_line":"        root_uuid \u003d root_summary.resource_provider.uuid"},{"line_number":3611,"context_line":""},{"line_number":3612,"context_line":"        # Using itertools.product, we get all the combinations of resource"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_063dff29","line":3609,"range":{"start_line":3609,"start_character":8,"end_line":3609,"end_character":41},"in_reply_to":"5f7c97a3_89a7edbd","updated":"2018-05-12 07:23:11.000000000","message":"\u003e Well look, if you leave this in place, then you can\u0027t remove the root IDs from L3572. \n\nGot it. Put back root_ids in L3572 within this patch\n\n\u003e we will have to be very careful to make sure we\u0027re using the right one.  And we have to account for cases where there use_same_provider\u003dFalse group only provides resources from sharing providers.\n\nRight. I think I get you because I have already looked into that case in https://review.openstack.org/#/c/567508/5, too. \n\nBut the point here is that this is good as long as we have the right anchor.\n\n\u003e I think what I\u0027m getting at is that this method needs to deal wholeheartedly with either trees-without-sharing or trees-with-sharing.  It can\u0027t go part way.\n\nLet\u0027s not settle for this part.\n\nTo get the right anchor, I modified _get_all_with_shared() function(which is very similar to the _get_anchor_** function you created) in https://review.openstack.org/#/c/567508/5.\nCould you review the patch, too?\n\nAnd thanks to that I now believe we can consolidate to one _alloc_candidates_multiple_provider function for both cases with sharing and no-sharing and that it results in less codes, which results in less bugs and less maintaining cost making our life better.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"405fda7c3e74e4ff11af1a960f463a2f162df53f","unresolved":false,"context_lines":[{"line_number":3606,"context_line":"        # AllocationRequestResource per requested resource class."},{"line_number":3607,"context_line":"        request_groups \u003d alloc_dict.values()"},{"line_number":3608,"context_line":""},{"line_number":3609,"context_line":"        root_summary \u003d summaries[root_id]"},{"line_number":3610,"context_line":"        root_uuid \u003d root_summary.resource_provider.uuid"},{"line_number":3611,"context_line":""},{"line_number":3612,"context_line":"        # Using itertools.product, we get all the combinations of resource"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_886b71b5","line":3609,"range":{"start_line":3609,"start_character":8,"end_line":3609,"end_character":41},"in_reply_to":"5f7c97a3_e2432612","updated":"2018-05-11 04:43:52.000000000","message":"That works when there the only combinations of AllocationRequestResource from the same root provider. Yup, that  means it\u0027s good and you can do it within this patch, but I\u0027m planning to merge sharing\u0026nested cases to this function. For those cases we can\u0027t retrieve anchor root uuids simply from AllocationRequestResource here, so I leave this for now.","commit_id":"48cdaa205b0e25f6fd0b0ddc05837f0ab382c93f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b2de541e411ca94dd0da003650c73499a3ffb0a","unresolved":false,"context_lines":[{"line_number":3548,"context_line":"    return alloc_requests, list(summaries.values())"},{"line_number":3549,"context_line":""},{"line_number":3550,"context_line":""},{"line_number":3551,"context_line":"def _alloc_candidates_multiple_providers(ctx, requested_resources, rp_tuples):"},{"line_number":3552,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3553,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3554,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_e9954141","line":3551,"range":{"start_line":3551,"start_character":22,"end_line":3551,"end_character":40},"updated":"2018-05-11 13:25:22.000000000","message":"nested_no_shared might be better.","commit_id":"130cb653539a8b66949202b3f32b0063666402f2"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3c824f83c16aaeff91b7949d3a0fd4311031f136","unresolved":false,"context_lines":[{"line_number":3548,"context_line":"    return alloc_requests, list(summaries.values())"},{"line_number":3549,"context_line":""},{"line_number":3550,"context_line":""},{"line_number":3551,"context_line":"def _alloc_candidates_multiple_providers(ctx, requested_resources, rp_tuples):"},{"line_number":3552,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3553,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3554,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_06541f52","line":3551,"range":{"start_line":3551,"start_character":22,"end_line":3551,"end_character":40},"in_reply_to":"5f7c97a3_e9954141","updated":"2018-05-12 07:23:11.000000000","message":"So far within this patch, okay. Done.","commit_id":"130cb653539a8b66949202b3f32b0063666402f2"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9b2de541e411ca94dd0da003650c73499a3ffb0a","unresolved":false,"context_lines":[{"line_number":3556,"context_line":"    requested by the user."},{"line_number":3557,"context_line":""},{"line_number":3558,"context_line":"    This is a code path to get results for a RequestGroup with"},{"line_number":3559,"context_line":"    use_same_provider\u003dTrue. In this scenario, we determine requests across"},{"line_number":3560,"context_line":"    multiple providers being aware of nested resource provider trees."},{"line_number":3561,"context_line":""},{"line_number":3562,"context_line":"    Currently this function should be used only for cases where no sharing"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_49431514","line":3559,"range":{"start_line":3559,"start_character":22,"end_line":3559,"end_character":26},"updated":"2018-05-11 13:25:22.000000000","message":"False","commit_id":"130cb653539a8b66949202b3f32b0063666402f2"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3c824f83c16aaeff91b7949d3a0fd4311031f136","unresolved":false,"context_lines":[{"line_number":3556,"context_line":"    requested by the user."},{"line_number":3557,"context_line":""},{"line_number":3558,"context_line":"    This is a code path to get results for a RequestGroup with"},{"line_number":3559,"context_line":"    use_same_provider\u003dTrue. In this scenario, we determine requests across"},{"line_number":3560,"context_line":"    multiple providers being aware of nested resource provider trees."},{"line_number":3561,"context_line":""},{"line_number":3562,"context_line":"    Currently this function should be used only for cases where no sharing"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_e656c35c","line":3559,"range":{"start_line":3559,"start_character":22,"end_line":3559,"end_character":26},"in_reply_to":"5f7c97a3_49431514","updated":"2018-05-12 07:23:11.000000000","message":"Oops... Thank you as always!","commit_id":"130cb653539a8b66949202b3f32b0063666402f2"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"9aabcd069dd89044622e708b8601e2963aa85ad7","unresolved":false,"context_lines":[{"line_number":2818,"context_line":""},{"line_number":2819,"context_line":""},{"line_number":2820,"context_line":"@db_api.api_context_manager.reader"},{"line_number":2821,"context_line":"def _get_usages_by_provider(ctx, rp_ids):"},{"line_number":2822,"context_line":"    \"\"\"Returns a row iterator of usage records grouped by resource provider ID"},{"line_number":2823,"context_line":"    and resource class ID for all resource providers"},{"line_number":2824,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_bc920556","line":2821,"range":{"start_line":2821,"start_character":3,"end_line":2821,"end_character":26},"updated":"2018-05-14 15:25:48.000000000","message":"Note for reviewers:\nThis change is not a main change, but needed because without this, requesting only nested resources results in lack of the root provider in provider summaries, which results in KeyError @L3584. (Even with this change the root providers that don\u0027t actually provide requested resource are filtered at last in later in _merge_candidates though...)\n\nSince this is consistent with Blueprint: placement-return-all-resource , this is good.","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"498d199ad8a3b0bf5c4596b894d10c6b28e7d4ef","unresolved":false,"context_lines":[{"line_number":2818,"context_line":""},{"line_number":2819,"context_line":""},{"line_number":2820,"context_line":"@db_api.api_context_manager.reader"},{"line_number":2821,"context_line":"def _get_usages_by_provider(ctx, rp_ids):"},{"line_number":2822,"context_line":"    \"\"\"Returns a row iterator of usage records grouped by resource provider ID"},{"line_number":2823,"context_line":"    and resource class ID for all resource providers"},{"line_number":2824,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_1ef2039b","line":2821,"range":{"start_line":2821,"start_character":3,"end_line":2821,"end_character":26},"in_reply_to":"5f7c97a3_a5e0a127","updated":"2018-05-15 01:06:03.000000000","message":"Well, fair point. But I want to answer to eric\u0027s question on PS5, Line 3609 within this patch to show that we can support resource requests on only non-root providers as well as requests with root providers.","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0374da63c892c99f86b2925f4527236a73df0b7b","unresolved":false,"context_lines":[{"line_number":2818,"context_line":""},{"line_number":2819,"context_line":""},{"line_number":2820,"context_line":"@db_api.api_context_manager.reader"},{"line_number":2821,"context_line":"def _get_usages_by_provider(ctx, rp_ids):"},{"line_number":2822,"context_line":"    \"\"\"Returns a row iterator of usage records grouped by resource provider ID"},{"line_number":2823,"context_line":"    and resource class ID for all resource providers"},{"line_number":2824,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_a5e0a127","line":2821,"range":{"start_line":2821,"start_character":3,"end_line":2821,"end_character":26},"in_reply_to":"5f7c97a3_bc920556","updated":"2018-05-14 20:21:06.000000000","message":"I definitely would have preferred to see this particular part of the patch separated into its own patch...","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0374da63c892c99f86b2925f4527236a73df0b7b","unresolved":false,"context_lines":[{"line_number":3281,"context_line":"    # JOIN ("},{"line_number":3282,"context_line":"    #   SELECT rp.root_provider_id"},{"line_number":3283,"context_line":"    #   FROM resource_providers AS rp"},{"line_number":3284,"context_line":"    #   LEFT JOIN resource_provider_traits AS rptt"},{"line_number":3285,"context_line":"    #   ON rp.id \u003d rptt.resource_provider_id"},{"line_number":3286,"context_line":"    #   WHERE rp.id IN ($RP_IDS_WITH_INV)"},{"line_number":3287,"context_line":"    #   AND rptt.trait_id IN ($REQUIRED_TRAIT_IDS)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_2a2a1e2b","line":3284,"range":{"start_line":3284,"start_character":7,"end_line":3284,"end_character":50},"updated":"2018-05-14 20:21:06.000000000","message":"this change wasn\u0027t necessary.","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"498d199ad8a3b0bf5c4596b894d10c6b28e7d4ef","unresolved":false,"context_lines":[{"line_number":3281,"context_line":"    # JOIN ("},{"line_number":3282,"context_line":"    #   SELECT rp.root_provider_id"},{"line_number":3283,"context_line":"    #   FROM resource_providers AS rp"},{"line_number":3284,"context_line":"    #   LEFT JOIN resource_provider_traits AS rptt"},{"line_number":3285,"context_line":"    #   ON rp.id \u003d rptt.resource_provider_id"},{"line_number":3286,"context_line":"    #   WHERE rp.id IN ($RP_IDS_WITH_INV)"},{"line_number":3287,"context_line":"    #   AND rptt.trait_id IN ($REQUIRED_TRAIT_IDS)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_7e7bf726","line":3284,"range":{"start_line":3284,"start_character":7,"end_line":3284,"end_character":50},"in_reply_to":"5f7c97a3_2a2a1e2b","updated":"2018-05-15 01:06:03.000000000","message":"This is needed for cases where forbidden traits specified without required traits.\n\nINNER JOIN here deletes rps without any traits from the table.\nBut \"rps without any traits\" should be a candidate for a request with \"forbidden_traits\u003dsomething\" without required traits.\n\nSo we should use OUTER JOIN to keep rps without any traits here.","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0374da63c892c99f86b2925f4527236a73df0b7b","unresolved":false,"context_lines":[{"line_number":3314,"context_line":""},{"line_number":3315,"context_line":"    if required_traits:"},{"line_number":3316,"context_line":"        # Only get the resource providers that have ALL the required traits,"},{"line_number":3317,"context_line":"        # so we need to GROUP BY the resource provider and ensure that the"},{"line_number":3318,"context_line":"        # COUNT(trait_id) is equal to the number of traits we are requiring"},{"line_number":3319,"context_line":"        cond.append(rptt.c.trait_id.in_(required_traits.values()))"},{"line_number":3320,"context_line":"        num_traits \u003d len(required_traits)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_8aefb2e7","line":3317,"range":{"start_line":3317,"start_character":32,"end_line":3317,"end_character":54},"updated":"2018-05-14 20:21:06.000000000","message":"might as well clean this up. should read \"GROUP BY the root provider\"","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"498d199ad8a3b0bf5c4596b894d10c6b28e7d4ef","unresolved":false,"context_lines":[{"line_number":3314,"context_line":""},{"line_number":3315,"context_line":"    if required_traits:"},{"line_number":3316,"context_line":"        # Only get the resource providers that have ALL the required traits,"},{"line_number":3317,"context_line":"        # so we need to GROUP BY the resource provider and ensure that the"},{"line_number":3318,"context_line":"        # COUNT(trait_id) is equal to the number of traits we are requiring"},{"line_number":3319,"context_line":"        cond.append(rptt.c.trait_id.in_(required_traits.values()))"},{"line_number":3320,"context_line":"        num_traits \u003d len(required_traits)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_1e76833f","line":3317,"range":{"start_line":3317,"start_character":32,"end_line":3317,"end_character":54},"in_reply_to":"5f7c97a3_8aefb2e7","updated":"2018-05-15 01:06:03.000000000","message":"Done. Thanks!","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0374da63c892c99f86b2925f4527236a73df0b7b","unresolved":false,"context_lines":[{"line_number":3539,"context_line":""},{"line_number":3540,"context_line":"def _alloc_candidates_nested_no_shared(ctx, requested_resources, rp_tuples):"},{"line_number":3541,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3542,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3543,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"},{"line_number":3544,"context_line":"    in the requested resources as well as ALL required traits that were"},{"line_number":3545,"context_line":"    requested by the user."}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_25a7b10d","line":3542,"range":{"start_line":3542,"start_character":51,"end_line":3542,"end_character":69},"updated":"2018-05-14 20:21:06.000000000","message":"s/resource providers/tuples of (rp_id, root_id, rc_id)/","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"498d199ad8a3b0bf5c4596b894d10c6b28e7d4ef","unresolved":false,"context_lines":[{"line_number":3539,"context_line":""},{"line_number":3540,"context_line":"def _alloc_candidates_nested_no_shared(ctx, requested_resources, rp_tuples):"},{"line_number":3541,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3542,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3543,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"},{"line_number":3544,"context_line":"    in the requested resources as well as ALL required traits that were"},{"line_number":3545,"context_line":"    requested by the user."}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_3e71ff44","line":3542,"range":{"start_line":3542,"start_character":51,"end_line":3542,"end_character":69},"in_reply_to":"5f7c97a3_25a7b10d","updated":"2018-05-15 01:06:03.000000000","message":"Done. Thanks!","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0374da63c892c99f86b2925f4527236a73df0b7b","unresolved":false,"context_lines":[{"line_number":3539,"context_line":""},{"line_number":3540,"context_line":"def _alloc_candidates_nested_no_shared(ctx, requested_resources, rp_tuples):"},{"line_number":3541,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3542,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3543,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"},{"line_number":3544,"context_line":"    in the requested resources as well as ALL required traits that were"},{"line_number":3545,"context_line":"    requested by the user."},{"line_number":3546,"context_line":""},{"line_number":3547,"context_line":"    This is a code path to get results for a RequestGroup with"},{"line_number":3548,"context_line":"    use_same_provider\u003dFalse. In this scenario, we determine requests across"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_45874d63","line":3545,"range":{"start_line":3542,"start_character":71,"end_line":3545,"end_character":26},"updated":"2018-05-14 20:21:06.000000000","message":"The supplied resource provider *trees* have ...","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"498d199ad8a3b0bf5c4596b894d10c6b28e7d4ef","unresolved":false,"context_lines":[{"line_number":3539,"context_line":""},{"line_number":3540,"context_line":"def _alloc_candidates_nested_no_shared(ctx, requested_resources, rp_tuples):"},{"line_number":3541,"context_line":"    \"\"\"Returns a tuple of (allocation requests, provider summaries) for a"},{"line_number":3542,"context_line":"    supplied set of requested resource amounts and resource providers. The"},{"line_number":3543,"context_line":"    supplied resource providers have capacity to satisfy ALL of the resources"},{"line_number":3544,"context_line":"    in the requested resources as well as ALL required traits that were"},{"line_number":3545,"context_line":"    requested by the user."},{"line_number":3546,"context_line":""},{"line_number":3547,"context_line":"    This is a code path to get results for a RequestGroup with"},{"line_number":3548,"context_line":"    use_same_provider\u003dFalse. In this scenario, we determine requests across"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_fe94e738","line":3545,"range":{"start_line":3542,"start_character":71,"end_line":3545,"end_character":26},"in_reply_to":"5f7c97a3_45874d63","updated":"2018-05-15 01:06:03.000000000","message":"Done. Thanks!","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":2840,"context_line":""},{"line_number":2841,"context_line":""},{"line_number":2842,"context_line":"@db_api.api_context_manager.reader"},{"line_number":2843,"context_line":"def _get_usages_by_provider(ctx, rp_ids):"},{"line_number":2844,"context_line":"    \"\"\"Returns a row iterator of usage records grouped by resource provider ID"},{"line_number":2845,"context_line":"    and resource class ID for all resource providers"},{"line_number":2846,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_0a0d7de7","line":2843,"updated":"2018-06-05 20:33:36.000000000","message":"little nit... you could have just made the rc_ids parameter optional and if not present, excluded the extra filter on resource_class_id IN (...).","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3249,"context_line":"            # If there\u0027s no providers that have one of the resource classes,"},{"line_number":3250,"context_line":"            # then we can short-circuit"},{"line_number":3251,"context_line":"            return []"},{"line_number":3252,"context_line":"        rc_provs_with_inv \u003d set((p[0], p[1], rc_id) for p in rc_provs_with_inv)"},{"line_number":3253,"context_line":"        rc_trees \u003d set(p[1] for p in rc_provs_with_inv)"},{"line_number":3254,"context_line":"        provs_with_inv |\u003d rc_provs_with_inv"},{"line_number":3255,"context_line":"        if trees_with_inv:"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_eaca816c","line":3252,"range":{"start_line":3252,"start_character":8,"end_line":3252,"end_character":79},"updated":"2018-06-05 20:33:36.000000000","message":"This is really confusing. You are redefining the rc_provs_with_inv variable from a set of (rp_id, root_rp_id) tuples to a set of (rp_id, root_rp_id, rc_id) tuples.\n\nWhy not create a new variable entirely instead of redefining the rc_provs_with_inv variable?","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3ae86844c40d960e30df90a499f20db6d4a5385b","unresolved":false,"context_lines":[{"line_number":3249,"context_line":"            # If there\u0027s no providers that have one of the resource classes,"},{"line_number":3250,"context_line":"            # then we can short-circuit"},{"line_number":3251,"context_line":"            return []"},{"line_number":3252,"context_line":"        rc_provs_with_inv \u003d set((p[0], p[1], rc_id) for p in rc_provs_with_inv)"},{"line_number":3253,"context_line":"        rc_trees \u003d set(p[1] for p in rc_provs_with_inv)"},{"line_number":3254,"context_line":"        provs_with_inv |\u003d rc_provs_with_inv"},{"line_number":3255,"context_line":"        if trees_with_inv:"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_0438093b","line":3252,"range":{"start_line":3252,"start_character":8,"end_line":3252,"end_character":79},"in_reply_to":"5f7c97a3_dc92ef9f","updated":"2018-06-06 05:49:17.000000000","message":"Done in https://review.openstack.org/#/c/572673/. This depends on the following _check_traits_for_alloc_request() patch.","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3d92b4669931ba2b40e3c2d5fe9eeb504d164b46","unresolved":false,"context_lines":[{"line_number":3249,"context_line":"            # If there\u0027s no providers that have one of the resource classes,"},{"line_number":3250,"context_line":"            # then we can short-circuit"},{"line_number":3251,"context_line":"            return []"},{"line_number":3252,"context_line":"        rc_provs_with_inv \u003d set((p[0], p[1], rc_id) for p in rc_provs_with_inv)"},{"line_number":3253,"context_line":"        rc_trees \u003d set(p[1] for p in rc_provs_with_inv)"},{"line_number":3254,"context_line":"        provs_with_inv |\u003d rc_provs_with_inv"},{"line_number":3255,"context_line":"        if trees_with_inv:"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_dc92ef9f","line":3252,"range":{"start_line":3252,"start_character":8,"end_line":3252,"end_character":79},"in_reply_to":"5f7c97a3_eaca816c","updated":"2018-06-06 00:34:09.000000000","message":"Ack, will do.","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3300,"context_line":"    #"},{"line_number":3301,"context_line":"    # SELECT outer_rp.id, outer_rp.root_provider_id"},{"line_number":3302,"context_line":"    # FROM resource_providers AS outer_rp"},{"line_number":3303,"context_line":"    # JOIN ("},{"line_number":3304,"context_line":"    #   SELECT rp.root_provider_id"},{"line_number":3305,"context_line":"    #   FROM resource_providers AS rp"},{"line_number":3306,"context_line":"    #   LEFT JOIN resource_provider_traits AS rptt"},{"line_number":3307,"context_line":"    #   ON rp.id \u003d rptt.resource_provider_id"},{"line_number":3308,"context_line":"    #   WHERE rp.id IN ($RP_IDS_WITH_INV)"},{"line_number":3309,"context_line":"    #   AND rptt.trait_id IN ($REQUIRED_TRAIT_IDS)"},{"line_number":3310,"context_line":"    #   GROUP BY rp.root_provider_id"},{"line_number":3311,"context_line":"    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS"},{"line_number":3312,"context_line":"    # ) AS trees_with_traits"},{"line_number":3313,"context_line":"    #  ON outer_rp.root_provider_id \u003d trees_with_traits.root_provider_id"},{"line_number":3314,"context_line":"    rp_ids_with_inv \u003d set(p[0] for p in provs_with_inv)"},{"line_number":3315,"context_line":""},{"line_number":3316,"context_line":"    # Build our inner subquery"},{"line_number":3317,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":3318,"context_line":"    rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rptt\")"},{"line_number":3319,"context_line":"    rpt_to_rptt \u003d sa.outerjoin("},{"line_number":3320,"context_line":"        rpt, rptt, rpt.c.id \u003d\u003d rptt.c.resource_provider_id)"},{"line_number":3321,"context_line":"    subq \u003d sa.select([rpt.c.root_provider_id])"},{"line_number":3322,"context_line":"    subq \u003d subq.select_from(rpt_to_rptt)"},{"line_number":3323,"context_line":"    cond \u003d [rpt.c.id.in_(rp_ids_with_inv)]"},{"line_number":3324,"context_line":""},{"line_number":3325,"context_line":"    # Tack on an additional WHERE clause for the derived table if we\u0027ve got"},{"line_number":3326,"context_line":"    # forbidden traits in the mix."},{"line_number":3327,"context_line":"    # TODO(jaypipes): This approach is not efficient. We could potentially"},{"line_number":3328,"context_line":"    # change _get_provider_ids_having_any_trait() to accept an optional rp_ids"},{"line_number":3329,"context_line":"    # parameter that would further winnow results to a set of resource provider"},{"line_number":3330,"context_line":"    # IDs (which we have here as we\u0027ve already looked up the providers that"},{"line_number":3331,"context_line":"    # have appropriate inventory capacity)"},{"line_number":3332,"context_line":"    if forbidden_traits:"},{"line_number":3333,"context_line":"        forbidden_rp_ids \u003d _get_provider_ids_having_any_trait("},{"line_number":3334,"context_line":"            ctx, forbidden_traits)"},{"line_number":3335,"context_line":"        cond.append(~rpt.c.id.in_(forbidden_rp_ids))"},{"line_number":3336,"context_line":""},{"line_number":3337,"context_line":"    if required_traits:"},{"line_number":3338,"context_line":"        # Only get the resource providers that have ALL the required traits,"},{"line_number":3339,"context_line":"        # so we need to GROUP BY the root provider and ensure that the"},{"line_number":3340,"context_line":"        # COUNT(trait_id) is equal to the number of traits we are requiring"},{"line_number":3341,"context_line":"        cond.append(rptt.c.trait_id.in_(required_traits.values()))"},{"line_number":3342,"context_line":"        num_traits \u003d len(required_traits)"},{"line_number":3343,"context_line":"        having_cond \u003d sa.func.count(sa.distinct(rptt.c.trait_id)) \u003d\u003d num_traits"},{"line_number":3344,"context_line":"        subq \u003d subq.having(having_cond)"},{"line_number":3345,"context_line":""},{"line_number":3346,"context_line":"    subq \u003d subq.where(sa.and_(*cond))"},{"line_number":3347,"context_line":"    subq \u003d subq.group_by(rpt.c.root_provider_id)"},{"line_number":3348,"context_line":"    trees_with_traits \u003d sa.alias(subq, name\u003d\"trees_with_traits\")"},{"line_number":3349,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_904d40c6","line":3346,"range":{"start_line":3303,"start_character":0,"end_line":3346,"end_character":37},"updated":"2018-06-05 20:33:36.000000000","message":"The more I looked at this, the more I realized that the forbidden_traits was not being handled properly within the subquery.\n\nYou changed the JOIN on resource_provider_traits to a LEFT JOIN, probably because when you made these changes, you found test_all_local() func test failed. :) And making the LEFT JOIN on resource_provider_traits seemed like the right fix.\n\nBut, it isn\u0027t really. The correct SQL that we\u0027re looking for is this:\n\n    # SELECT outer_rp.id, outer_rp.root_provider_id\n    # FROM resource_providers AS outer_rp\n    # JOIN (\n    #   SELECT rp.root_provider_id\n    #   FROM resource_providers AS rp\n    #   # Only if we have required traits...\n    #   INNER JOIN resource_provider_traits AS rptt\n    #   ON rp.id \u003d rptt.resource_provider_id\n    #   AND rptt.trait_id IN ($REQUIRED_TRAIT_IDS)\n    #   # Only if we have forbidden_traits...\n    #   LEFT JOIN resource_provider_traits AS rptt_forbid\n    #   ON rp.id \u003d rptt_forbid.resource_provider_id\n    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS)\n    #   WHERE rp.id IN ($RP_IDS_WITH_INV)\n    #   # Only if we have forbidden traits...\n    #   AND rptt_forbid.resource_provider_id IS NULL\n    #   GROUP BY rp.root_provider_id\n    #   # Only if have required traits...\n    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS\n    # ) AS trees_with_traits\n    #  ON outer_rp.root_provider_id \u003d trees_with_traits.root_provider_id\n\nNote that we need potentially two joins to resource_provider_traits, with only the second join being a LEFT JOIN (with a WHERE rptt_forbid.resource_provider IS NULL clause) and only when forbidden traits are present.\n\nI rewrote the SQL generating code here to be the following:\n\n    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")\n    cond \u003d [rpt.c.id.in_(rp_ids_with_inv)]\n    subq \u003d sa.select([rpt.c.root_provider_id])\n    subq_join \u003d None\n    if required_traits:\n        rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rptt\")\n        rpt_to_rptt \u003d sa.join(\n            rpt, rptt, sa.and_(\n                rpt.c.id \u003d\u003d rptt.c.resource_provider_id,\n                rptt.c.trait_id.in_(required_traits.values())))\n        subq_join \u003d rpt_to_rptt\n        # Only get the resource providers that have ALL the required traits,\n        # so we need to GROUP BY the root provider and ensure that the\n        # COUNT(trait_id) is equal to the number of traits we are requiring\n        num_traits \u003d len(required_traits)\n        having_cond \u003d sa.func.count(sa.distinct(rptt.c.trait_id)) \u003d\u003d num_traits\n        subq \u003d subq.having(having_cond)\n\n    # Tack on an additional LEFT JOIN clause inside the derived table if we\u0027ve\n    # got forbidden traits in the mix.\n    if forbidden_traits:\n        rptt_forbid \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rptt_forbid\")\n        join_to \u003d rpt\n        if subq_join is not None:\n            join_to \u003d subq_join\n        rpt_to_rptt_forbid \u003d sa.outerjoin(\n            join_to, rptt_forbid, sa.and_(\n                rpt.c.id \u003d\u003d rptt_forbid.c.resource_provider_id,\n                rptt_forbid.c.trait_id.in_(forbidden_traits.values())))\n        cond.append(rptt_forbid.c.resource_provider_id.is_(None))\n        subq_join \u003d rpt_to_rptt_forbid\n\n    subq \u003d subq.select_from(subq_join)\n    subq \u003d subq.where(sa.and_(*cond))\n    subq \u003d subq.group_by(rpt.c.root_provider_id)\n    trees_with_traits \u003d sa.alias(subq, name\u003d\"trees_with_traits\")\n\n    outer_rps \u003d sa.alias(_RP_TBL, name\u003d\"outer_rps\")\n    outer_to_subq \u003d sa.join(\n        outer_rps, trees_with_traits,\n        outer_rps.c.root_provider_id \u003d\u003d trees_with_traits.c.root_provider_id)\n    sel \u003d sa.select([outer_rps.c.id, outer_rps.c.root_provider_id])\n    sel \u003d sel.select_from(outer_to_subq)\n\n    res \u003d ctx.session.execute(sel).fetchall()\n\nAnd ran all the functional tests successfully. You will note that we get rid of the inefficient use of _get_provider_ids_having_any_trait() by instead doing this (only when forbidden traits are requested):\n\n LEFT JOIN resource_provider_traits AS rptt_forbid\n ON rpt.id \u003d rptt_forbid.resource_provider_id\n AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAITS)\n WHERE rptt_forbid.resource_provider_id IS NULL\n\nthe above is both more efficient (only processes a small amount of records) and allows the more efficient INNER JOIN to remain against the rptt table that we use to get the providers containing required traits.\n\nAnother benefit to this is that we no longer do the inefficient HAVING clause when there are no required traits (only forbidden traits).","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3d92b4669931ba2b40e3c2d5fe9eeb504d164b46","unresolved":false,"context_lines":[{"line_number":3300,"context_line":"    #"},{"line_number":3301,"context_line":"    # SELECT outer_rp.id, outer_rp.root_provider_id"},{"line_number":3302,"context_line":"    # FROM resource_providers AS outer_rp"},{"line_number":3303,"context_line":"    # JOIN ("},{"line_number":3304,"context_line":"    #   SELECT rp.root_provider_id"},{"line_number":3305,"context_line":"    #   FROM resource_providers AS rp"},{"line_number":3306,"context_line":"    #   LEFT JOIN resource_provider_traits AS rptt"},{"line_number":3307,"context_line":"    #   ON rp.id \u003d rptt.resource_provider_id"},{"line_number":3308,"context_line":"    #   WHERE rp.id IN ($RP_IDS_WITH_INV)"},{"line_number":3309,"context_line":"    #   AND rptt.trait_id IN ($REQUIRED_TRAIT_IDS)"},{"line_number":3310,"context_line":"    #   GROUP BY rp.root_provider_id"},{"line_number":3311,"context_line":"    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS"},{"line_number":3312,"context_line":"    # ) AS trees_with_traits"},{"line_number":3313,"context_line":"    #  ON outer_rp.root_provider_id \u003d trees_with_traits.root_provider_id"},{"line_number":3314,"context_line":"    rp_ids_with_inv \u003d set(p[0] for p in provs_with_inv)"},{"line_number":3315,"context_line":""},{"line_number":3316,"context_line":"    # Build our inner subquery"},{"line_number":3317,"context_line":"    rpt \u003d sa.alias(_RP_TBL, name\u003d\"rp\")"},{"line_number":3318,"context_line":"    rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rptt\")"},{"line_number":3319,"context_line":"    rpt_to_rptt \u003d sa.outerjoin("},{"line_number":3320,"context_line":"        rpt, rptt, rpt.c.id \u003d\u003d rptt.c.resource_provider_id)"},{"line_number":3321,"context_line":"    subq \u003d sa.select([rpt.c.root_provider_id])"},{"line_number":3322,"context_line":"    subq \u003d subq.select_from(rpt_to_rptt)"},{"line_number":3323,"context_line":"    cond \u003d [rpt.c.id.in_(rp_ids_with_inv)]"},{"line_number":3324,"context_line":""},{"line_number":3325,"context_line":"    # Tack on an additional WHERE clause for the derived table if we\u0027ve got"},{"line_number":3326,"context_line":"    # forbidden traits in the mix."},{"line_number":3327,"context_line":"    # TODO(jaypipes): This approach is not efficient. We could potentially"},{"line_number":3328,"context_line":"    # change _get_provider_ids_having_any_trait() to accept an optional rp_ids"},{"line_number":3329,"context_line":"    # parameter that would further winnow results to a set of resource provider"},{"line_number":3330,"context_line":"    # IDs (which we have here as we\u0027ve already looked up the providers that"},{"line_number":3331,"context_line":"    # have appropriate inventory capacity)"},{"line_number":3332,"context_line":"    if forbidden_traits:"},{"line_number":3333,"context_line":"        forbidden_rp_ids \u003d _get_provider_ids_having_any_trait("},{"line_number":3334,"context_line":"            ctx, forbidden_traits)"},{"line_number":3335,"context_line":"        cond.append(~rpt.c.id.in_(forbidden_rp_ids))"},{"line_number":3336,"context_line":""},{"line_number":3337,"context_line":"    if required_traits:"},{"line_number":3338,"context_line":"        # Only get the resource providers that have ALL the required traits,"},{"line_number":3339,"context_line":"        # so we need to GROUP BY the root provider and ensure that the"},{"line_number":3340,"context_line":"        # COUNT(trait_id) is equal to the number of traits we are requiring"},{"line_number":3341,"context_line":"        cond.append(rptt.c.trait_id.in_(required_traits.values()))"},{"line_number":3342,"context_line":"        num_traits \u003d len(required_traits)"},{"line_number":3343,"context_line":"        having_cond \u003d sa.func.count(sa.distinct(rptt.c.trait_id)) \u003d\u003d num_traits"},{"line_number":3344,"context_line":"        subq \u003d subq.having(having_cond)"},{"line_number":3345,"context_line":""},{"line_number":3346,"context_line":"    subq \u003d subq.where(sa.and_(*cond))"},{"line_number":3347,"context_line":"    subq \u003d subq.group_by(rpt.c.root_provider_id)"},{"line_number":3348,"context_line":"    trees_with_traits \u003d sa.alias(subq, name\u003d\"trees_with_traits\")"},{"line_number":3349,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_5cbfbfcf","line":3346,"range":{"start_line":3303,"start_character":0,"end_line":3346,"end_character":37},"in_reply_to":"5f7c97a3_904d40c6","updated":"2018-06-06 00:34:09.000000000","message":"Yup, I was going to fix my fix (always use Left JOIN) here by moving forbidden traits clause into _get_providers_with_resource() function... but the approach you wrote here seems better and clearer especially in the point that we can get rid of _get_provider_ids_having_any_trait, which is very efficient.\n\nThank you so much for review in details.","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3359,"context_line":"    rp_tuples_with_trait \u003d [(rp_id, root_id) for rp_id, root_id in res]"},{"line_number":3360,"context_line":""},{"line_number":3361,"context_line":"    ret \u003d [rp_tuple for rp_tuple in provs_with_inv if ("},{"line_number":3362,"context_line":"        rp_tuple[0], rp_tuple[1]) in rp_tuples_with_trait]"},{"line_number":3363,"context_line":""},{"line_number":3364,"context_line":"    return ret"},{"line_number":3365,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_0a0c9d92","line":3362,"updated":"2018-06-05 20:33:36.000000000","message":"++","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3523,"context_line":"    \"\"\""},{"line_number":3524,"context_line":"    if not rps:"},{"line_number":3525,"context_line":"        return [], []"},{"line_number":3526,"context_line":"    # Grab usage summaries for each provider"},{"line_number":3527,"context_line":"    rp_ids \u003d set(p[0] for p in rps)"},{"line_number":3528,"context_line":"    usages \u003d _get_usages_by_provider(ctx, rp_ids)"},{"line_number":3529,"context_line":""},{"line_number":3530,"context_line":"    # Get a dict, keyed by resource provider internal ID, of trait string names"},{"line_number":3531,"context_line":"    # that provider has associated with it"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_2a5f1973","line":3528,"range":{"start_line":3526,"start_character":0,"end_line":3528,"end_character":49},"updated":"2018-06-05 20:33:36.000000000","message":"I\u0027m still not convinced that this belonged in this particular patch. Returning the usage information for all resources (regardless of whether the resource class was being requested) is a different problem than solving allocation candidates for nested providers...","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3567,"context_line":"    well as ALL required traits that were requested by the user."},{"line_number":3568,"context_line":""},{"line_number":3569,"context_line":"    This is a code path to get results for a RequestGroup with"},{"line_number":3570,"context_line":"    use_same_provider\u003dFalse. In this scenario, we determine requests across"},{"line_number":3571,"context_line":"    multiple providers being aware of nested resource provider trees."},{"line_number":3572,"context_line":""},{"line_number":3573,"context_line":"    Currently this function should be used only for cases where no sharing"},{"line_number":3574,"context_line":"    providers exist in the system for any requested resource. If sharing"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_0ae81d7a","line":3571,"range":{"start_line":3570,"start_character":47,"end_line":3571,"end_character":68},"updated":"2018-06-05 20:33:36.000000000","message":"suggested rewording:\n\n\"In this scenario, we are able to use multiple providers within the same provider tree to satisfy different resources involved in a single request group.\"","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3d92b4669931ba2b40e3c2d5fe9eeb504d164b46","unresolved":false,"context_lines":[{"line_number":3567,"context_line":"    well as ALL required traits that were requested by the user."},{"line_number":3568,"context_line":""},{"line_number":3569,"context_line":"    This is a code path to get results for a RequestGroup with"},{"line_number":3570,"context_line":"    use_same_provider\u003dFalse. In this scenario, we determine requests across"},{"line_number":3571,"context_line":"    multiple providers being aware of nested resource provider trees."},{"line_number":3572,"context_line":""},{"line_number":3573,"context_line":"    Currently this function should be used only for cases where no sharing"},{"line_number":3574,"context_line":"    providers exist in the system for any requested resource. If sharing"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_fc7b2b78","line":3571,"range":{"start_line":3570,"start_character":47,"end_line":3571,"end_character":68},"in_reply_to":"5f7c97a3_0ae81d7a","updated":"2018-06-06 00:34:09.000000000","message":"Ack will do.","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3ae86844c40d960e30df90a499f20db6d4a5385b","unresolved":false,"context_lines":[{"line_number":3567,"context_line":"    well as ALL required traits that were requested by the user."},{"line_number":3568,"context_line":""},{"line_number":3569,"context_line":"    This is a code path to get results for a RequestGroup with"},{"line_number":3570,"context_line":"    use_same_provider\u003dFalse. In this scenario, we determine requests across"},{"line_number":3571,"context_line":"    multiple providers being aware of nested resource provider trees."},{"line_number":3572,"context_line":""},{"line_number":3573,"context_line":"    Currently this function should be used only for cases where no sharing"},{"line_number":3574,"context_line":"    providers exist in the system for any requested resource. If sharing"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_e40b8d7a","line":3571,"range":{"start_line":3570,"start_character":47,"end_line":3571,"end_character":68},"in_reply_to":"5f7c97a3_fc7b2b78","updated":"2018-06-06 05:49:17.000000000","message":"Done in https://review.openstack.org/#/c/572673/.","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3585,"context_line":"    if not rp_tuples:"},{"line_number":3586,"context_line":"        return [], []"},{"line_number":3587,"context_line":""},{"line_number":3588,"context_line":"    # Grab usage summaries for each provider including root providers"},{"line_number":3589,"context_line":"    # that don\u0027t provide the requested resources"},{"line_number":3590,"context_line":"    rp_ids \u003d set(p[0] for p in rp_tuples) | set(p[1] for p in rp_tuples)"},{"line_number":3591,"context_line":"    usages \u003d _get_usages_by_provider(ctx, rp_ids)"},{"line_number":3592,"context_line":""},{"line_number":3593,"context_line":"    # Get a dict, keyed by resource provider internal ID, of trait string names"},{"line_number":3594,"context_line":"    # that provider has associated with it"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_6a98b11a","line":3591,"range":{"start_line":3588,"start_character":0,"end_line":3591,"end_character":49},"updated":"2018-06-05 20:33:36.000000000","message":"Again, this isn\u0027t really relevant to what you\u0027re trying to solve in this particular patch. It could easily go in a followup patch that is focused on changing the behaviour from only returning provider information for those providers that are providing some resources in one of the allocation candidates.","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3632,"context_line":"            alloc_requests.append("},{"line_number":3633,"context_line":"                AllocationRequest(ctx, resource_requests\u003dlist(res_requests),"},{"line_number":3634,"context_line":"                                  anchor_root_provider_uuid\u003droot_uuid)"},{"line_number":3635,"context_line":"            )"},{"line_number":3636,"context_line":"    return alloc_requests, list(summaries.values())"},{"line_number":3637,"context_line":""},{"line_number":3638,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_0d07d71a","line":3635,"updated":"2018-06-05 20:33:36.000000000","message":"++ all of the above looks fine. nice work.","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"be3e9036ec3db530804541a1e3e4b982235d4811","unresolved":false,"context_lines":[{"line_number":3679,"context_line":"    if not all_rp_ids:"},{"line_number":3680,"context_line":"        return [], []"},{"line_number":3681,"context_line":""},{"line_number":3682,"context_line":"    # Grab usage summaries for each provider (local or sharing)"},{"line_number":3683,"context_line":"    usages \u003d _get_usages_by_provider(ctx, all_rp_ids)"},{"line_number":3684,"context_line":""},{"line_number":3685,"context_line":"    # Get a dict, keyed by resource provider internal ID, of trait string names"},{"line_number":3686,"context_line":"    # that provider has associated with it"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_ed361be8","line":3683,"range":{"start_line":3682,"start_character":0,"end_line":3683,"end_character":53},"updated":"2018-06-05 20:33:36.000000000","message":"unrelated to this patch. :)","commit_id":"737c897e48f51a22434c1575d5f9402284de3c2d"}],"nova/tests/functional/api/openstack/placement/db/test_allocation_candidates.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0374da63c892c99f86b2925f4527236a73df0b7b","unresolved":false,"context_lines":[{"line_number":1756,"context_line":"            \u0027cn_numa0_pf0\u0027: set([]),"},{"line_number":1757,"context_line":"            \u0027cn_numa1_pf1\u0027: set([os_traits.HW_NIC_OFFLOAD_GENEVE]),"},{"line_number":1758,"context_line":"        }"},{"line_number":1759,"context_line":"        self._validate_provider_summary_traits(expected, alloc_cands)"},{"line_number":1760,"context_line":""},{"line_number":1761,"context_line":"        # Now consume all the inventory of SRIOV_NET_VF on the second physical"},{"line_number":1762,"context_line":"        # function (the one with HW_NIC_OFFLOAD_GENEVE associated with it) and"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f7c97a3_4563cdb2","line":1759,"updated":"2018-05-14 20:21:06.000000000","message":"++ good cleanup/fixes in the tests.","commit_id":"44c5b4703d73ffdb9ecfb6799a778f36f5d915b0"}]}