)]}'
{"nova/objects/resource_provider.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":2425,"context_line":""},{"line_number":2426,"context_line":""},{"line_number":2427,"context_line":"@db_api.api_context_manager.reader"},{"line_number":2428,"context_line":"def _get_provider_ids_having_any_trait(ctx, traits):"},{"line_number":2429,"context_line":"    \"\"\"Returns a list of resource provider internal IDs that have ANY of the"},{"line_number":2430,"context_line":"    supplied traits."},{"line_number":2431,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_6ae3a835","line":2428,"range":{"start_line":2428,"start_character":4,"end_line":2428,"end_character":38},"updated":"2017-11-02 12:55:23.000000000","message":"Okay, I just started reviewing this patch, so maybe I\u0027ll answer my own question later on, but...\n\nWhat good is this method?  It will be quite possible to wind up with a legitimate candidate where all of the requested traits are subsumed by a proper subset of the RPs, and the remaining RPs have *none* of the requested traits.\n\nIf this method were mapping RPs to a list of their traits, I could see where that might be useful.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":2777,"context_line":""},{"line_number":2778,"context_line":"    # Get a dict, keyed by resource provider internal ID, of ProviderSummary"},{"line_number":2779,"context_line":"    # objects for all providers involved in the request"},{"line_number":2780,"context_line":"    # TODO(jaypipes): Handle traits for sharing providers scenario"},{"line_number":2781,"context_line":"    summaries \u003d _build_provider_summaries(ctx, usages, prov_traits)"},{"line_number":2782,"context_line":""},{"line_number":2783,"context_line":"    # Next, build up a list of allocation requests. These allocation requests"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_ca429c0f","line":2780,"range":{"start_line":2780,"start_character":4,"end_line":2780,"end_character":66},"updated":"2017-11-02 12:55:23.000000000","message":"This can go now","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":2777,"context_line":""},{"line_number":2778,"context_line":"    # Get a dict, keyed by resource provider internal ID, of ProviderSummary"},{"line_number":2779,"context_line":"    # objects for all providers involved in the request"},{"line_number":2780,"context_line":"    # TODO(jaypipes): Handle traits for sharing providers scenario"},{"line_number":2781,"context_line":"    summaries \u003d _build_provider_summaries(ctx, usages, prov_traits)"},{"line_number":2782,"context_line":""},{"line_number":2783,"context_line":"    # Next, build up a list of allocation requests. These allocation requests"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_ae41d63b","line":2780,"range":{"start_line":2780,"start_character":4,"end_line":2780,"end_character":66},"in_reply_to":"3f4b6375_ca429c0f","updated":"2017-11-14 21:08:04.000000000","message":"Done","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"1f5c388fd520ce24e4d5f0c8cc100e13f5f8c6f4","unresolved":false,"context_lines":[{"line_number":2814,"context_line":"            ]"},{"line_number":2815,"context_line":"        )"},{"line_number":2816,"context_line":""},{"line_number":2817,"context_line":"        # We need to figure out which traits are NOT provided by the \"local\""},{"line_number":2818,"context_line":"        # provider and that would need to be provided by the sharing"},{"line_number":2819,"context_line":"        # provider(s)"},{"line_number":2820,"context_line":"        need_traits \u003d set(required_traits)"},{"line_number":2821,"context_line":"        missing_traits \u003d set()"},{"line_number":2822,"context_line":"        if required_traits:"},{"line_number":2823,"context_line":"            local_traits \u003d set(prov_traits.get(rp_id, []))"},{"line_number":2824,"context_line":"            missing_traits \u003d need_traits - local_traits"},{"line_number":2825,"context_line":""},{"line_number":2826,"context_line":"        # Determine if the non-sharing provider actually has all the"},{"line_number":2827,"context_line":"        # resources requested. If not, we need to add an AllocationRequest"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_f41428a2","line":2824,"range":{"start_line":2817,"start_character":8,"end_line":2824,"end_character":55},"updated":"2017-11-02 03:18:50.000000000","message":"this is similar to my third method, fetch all the traits, and figure out which RP has enough traits in the python code. But you need to remove the method \u0027_get_provider_ids_having_any_trait\u0027 and the filtering at line 3061.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":2830,"context_line":"        if has_all:"},{"line_number":2831,"context_line":"            # If this resource provider has all the requested resources, then"},{"line_number":2832,"context_line":"            # it must also have ALL required traits"},{"line_number":2833,"context_line":"            if missing_traits:"},{"line_number":2834,"context_line":"                continue"},{"line_number":2835,"context_line":"            req \u003d _allocation_request_for_provider(ctx, requested_resources,"},{"line_number":2836,"context_line":"                                                   rp_uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_4afc6cb4","line":2833,"updated":"2017-11-02 12:55:23.000000000","message":"Thinking a debug log wouldn\u0027t go amiss here.  \"Excluding non-sharing provider %s: it has all resources but is missing traits %s.\"","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":2877,"context_line":"                shared_rp_id \u003d None"},{"line_number":2878,"context_line":"                for rp_id, summary in summaries.items():"},{"line_number":2879,"context_line":"                    if summary.resource_provider.uuid \u003d\u003d sharing_rp_uuid:"},{"line_number":2880,"context_line":"                        shared_rp_id \u003d rp_id"},{"line_number":2881,"context_line":"                share_prov_traits \u003d prov_traits.get(shared_rp_id, [])"},{"line_number":2882,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2883,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_ea42f867","line":2880,"updated":"2017-11-02 12:55:23.000000000","message":"and break?","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":2877,"context_line":"                shared_rp_id \u003d None"},{"line_number":2878,"context_line":"                for rp_id, summary in summaries.items():"},{"line_number":2879,"context_line":"                    if summary.resource_provider.uuid \u003d\u003d sharing_rp_uuid:"},{"line_number":2880,"context_line":"                        shared_rp_id \u003d rp_id"},{"line_number":2881,"context_line":"                share_prov_traits \u003d prov_traits.get(shared_rp_id, [])"},{"line_number":2882,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2883,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_4e5022d5","line":2880,"in_reply_to":"3f4b6375_ea42f867","updated":"2017-11-14 21:08:04.000000000","message":"Done","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":2881,"context_line":"                share_prov_traits \u003d prov_traits.get(shared_rp_id, [])"},{"line_number":2882,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2883,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":2884,"context_line":"            if still_missing_traits:"},{"line_number":2885,"context_line":"                continue"},{"line_number":2886,"context_line":""},{"line_number":2887,"context_line":"            resource_requests \u003d non_shared_requests + shared_requests"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_8a74a4d0","line":2884,"updated":"2017-11-02 12:55:23.000000000","message":"debug log","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":2881,"context_line":"                share_prov_traits \u003d prov_traits.get(shared_rp_id, [])"},{"line_number":2882,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2883,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":2884,"context_line":"            if still_missing_traits:"},{"line_number":2885,"context_line":"                continue"},{"line_number":2886,"context_line":""},{"line_number":2887,"context_line":"            resource_requests \u003d non_shared_requests + shared_requests"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_2e0606d0","line":2884,"in_reply_to":"3f4b6375_8a74a4d0","updated":"2017-11-14 21:08:04.000000000","message":"Done","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"1f5c388fd520ce24e4d5f0c8cc100e13f5f8c6f4","unresolved":false,"context_lines":[{"line_number":2882,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2883,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":2884,"context_line":"            if still_missing_traits:"},{"line_number":2885,"context_line":"                continue"},{"line_number":2886,"context_line":""},{"line_number":2887,"context_line":"            resource_requests \u003d non_shared_requests + shared_requests"},{"line_number":2888,"context_line":"            req \u003d AllocationRequest(ctx, resource_requests\u003dresource_requests)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_d4f5ac58","line":2885,"updated":"2017-11-02 03:18:50.000000000","message":"My second method, actually can spread the trait filtering logic in the whole of this \u0027_get_by_filters\u0027 method. I feel that make this method even more hard to read. so you can see my second method, put all the traits logic together, considering the shared situation in the begin.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":2930,"context_line":"    res \u003d collections.defaultdict(list)"},{"line_number":2931,"context_line":"    for r in ctx.session.execute(sel):"},{"line_number":2932,"context_line":"        res[r[0]].append(r[1])"},{"line_number":2933,"context_line":""},{"line_number":2934,"context_line":"    return res"},{"line_number":2935,"context_line":""},{"line_number":2936,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_254e016f","line":2933,"updated":"2017-11-02 12:55:23.000000000","message":"gasp, unrelated whitespace change","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":2930,"context_line":"    res \u003d collections.defaultdict(list)"},{"line_number":2931,"context_line":"    for r in ctx.session.execute(sel):"},{"line_number":2932,"context_line":"        res[r[0]].append(r[1])"},{"line_number":2933,"context_line":""},{"line_number":2934,"context_line":"    return res"},{"line_number":2935,"context_line":""},{"line_number":2936,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_6e363ee9","line":2933,"in_reply_to":"3f4b6375_254e016f","updated":"2017-11-14 21:08:04.000000000","message":"Done","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8fb12cd7a4e0a35deeaca553d41865251a7cbe97","unresolved":false,"context_lines":[{"line_number":3058,"context_line":""},{"line_number":3059,"context_line":"        # We can further filter the set resource providers that don\u0027t match any"},{"line_number":3060,"context_line":"        # required trait..."},{"line_number":3061,"context_line":"        if trait_map:"},{"line_number":3062,"context_line":"            have_traits \u003d set(trait_rps)"},{"line_number":3063,"context_line":"            rp_ids \u0026\u003d have_traits"},{"line_number":3064,"context_line":""},{"line_number":3065,"context_line":"        return _alloc_candidates_with_shared(context, resources, trait_map,"},{"line_number":3066,"context_line":"                                             rp_ids, sharing_providers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_8edf2639","line":3063,"range":{"start_line":3061,"start_character":9,"end_line":3063,"end_character":33},"updated":"2017-11-02 02:14:44.000000000","message":"This doesn\u0027t works for this case:\n\nWe have compute nodes connnect to the shared resource provider, the shared resource provider has the SSD trait.\n\nIf only request the SSD trait, this will filter the RPs of compute nodes out.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":3058,"context_line":""},{"line_number":3059,"context_line":"        # We can further filter the set resource providers that don\u0027t match any"},{"line_number":3060,"context_line":"        # required trait..."},{"line_number":3061,"context_line":"        if trait_map:"},{"line_number":3062,"context_line":"            have_traits \u003d set(trait_rps)"},{"line_number":3063,"context_line":"            rp_ids \u0026\u003d have_traits"},{"line_number":3064,"context_line":""},{"line_number":3065,"context_line":"        return _alloc_candidates_with_shared(context, resources, trait_map,"},{"line_number":3066,"context_line":"                                             rp_ids, sharing_providers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_ae955642","line":3063,"range":{"start_line":3061,"start_character":9,"end_line":3063,"end_character":33},"in_reply_to":"3f4b6375_2538419e","updated":"2017-11-14 21:08:04.000000000","message":"The case Alex highlights at the top of this comment chain is represented by agg5 in test_multiple_aggregates_with_traits - which is indeed missing from the result.  However, removing this block doesn\u0027t change the test results - so those guys are already getting filtered out some other where.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"444d093fe7cf536a913b08fa09aca90c918100ff","unresolved":false,"context_lines":[{"line_number":3058,"context_line":""},{"line_number":3059,"context_line":"        # We can further filter the set resource providers that don\u0027t match any"},{"line_number":3060,"context_line":"        # required trait..."},{"line_number":3061,"context_line":"        if trait_map:"},{"line_number":3062,"context_line":"            have_traits \u003d set(trait_rps)"},{"line_number":3063,"context_line":"            rp_ids \u0026\u003d have_traits"},{"line_number":3064,"context_line":""},{"line_number":3065,"context_line":"        return _alloc_candidates_with_shared(context, resources, trait_map,"},{"line_number":3066,"context_line":"                                             rp_ids, sharing_providers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_6e3e2a2a","line":3063,"range":{"start_line":3061,"start_character":9,"end_line":3063,"end_character":33},"in_reply_to":"3f4b6375_8edf2639","updated":"2017-11-02 02:25:58.000000000","message":"Actually, I tried three methods:\n\n* One SQL just like \u0027_get_all_with_shared\u0027 consider the shared resource provider situation: https://review.openstack.org/#/c/489206/, you suggested use the \u0027any trait\u0027 method\n\n* Using the \u0027any trait\u0027 method, and consider the shared resource provider situation in the python code https://review.openstack.org/#/c/479766/10/nova/objects/resource_provider.py, looks like we can put the code of building the mappings between shared and non-shared RPs into a common method. But that still may make this whole path for shared case complex more. That is the reason I\u0027m going to refactor this whole method.\n\n* In my refactor series, I just fetch all traits for each candidate RPs, and then we filter out RP doesn\u0027t have enough request traits in the python code https://review.openstack.org/#/c/511184/. The reason for why I\u0027m doing this is that we need to fetch all traits for building provider summary anyway.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":3058,"context_line":""},{"line_number":3059,"context_line":"        # We can further filter the set resource providers that don\u0027t match any"},{"line_number":3060,"context_line":"        # required trait..."},{"line_number":3061,"context_line":"        if trait_map:"},{"line_number":3062,"context_line":"            have_traits \u003d set(trait_rps)"},{"line_number":3063,"context_line":"            rp_ids \u0026\u003d have_traits"},{"line_number":3064,"context_line":""},{"line_number":3065,"context_line":"        return _alloc_candidates_with_shared(context, resources, trait_map,"},{"line_number":3066,"context_line":"                                             rp_ids, sharing_providers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_2538419e","line":3063,"range":{"start_line":3061,"start_character":9,"end_line":3063,"end_character":33},"in_reply_to":"3f4b6375_8edf2639","updated":"2017-11-02 12:55:23.000000000","message":"Agree - this is what I was talking about in my comment on _get_provider_ids_having_any_trait at the top.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"bdf14bd248c6b0d7c275e124b505a0652a2b3a19","unresolved":false,"context_lines":[{"line_number":2447,"context_line":"    :raise ValueError: If traits is empty or None."},{"line_number":2448,"context_line":"    \"\"\""},{"line_number":2449,"context_line":"    if not traits:"},{"line_number":2450,"context_line":"        raise ValueError(\u0027traits must not be empty\u0027)"},{"line_number":2451,"context_line":""},{"line_number":2452,"context_line":"    rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rpt\")"},{"line_number":2453,"context_line":"    sel \u003d sa.select([rptt.c.resource_provider_id])"}],"source_content_type":"text/x-python","patch_set":12,"id":"1f485f77_cbc83223","line":2450,"range":{"start_line":2450,"start_character":25,"end_line":2450,"end_character":51},"updated":"2017-11-17 04:20:19.000000000","message":"nit: Add a maker function _() .","commit_id":"28e0d283c5f42f252dbda3c5c093f05e9fab44ab"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6852a01b608e101364efb1d9e6fbd186a3f18f6d","unresolved":false,"context_lines":[{"line_number":2447,"context_line":"    :raise ValueError: If traits is empty or None."},{"line_number":2448,"context_line":"    \"\"\""},{"line_number":2449,"context_line":"    if not traits:"},{"line_number":2450,"context_line":"        raise ValueError(\u0027traits must not be empty\u0027)"},{"line_number":2451,"context_line":""},{"line_number":2452,"context_line":"    rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rpt\")"},{"line_number":2453,"context_line":"    sel \u003d sa.select([rptt.c.resource_provider_id])"}],"source_content_type":"text/x-python","patch_set":12,"id":"1f485f77_336726ba","line":2450,"range":{"start_line":2450,"start_character":25,"end_line":2450,"end_character":51},"in_reply_to":"1f485f77_aac96759","updated":"2017-11-17 22:04:41.000000000","message":"https://review.openstack.org/#/c/521189/","commit_id":"28e0d283c5f42f252dbda3c5c093f05e9fab44ab"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c0f49a047f5cd14f4fae99817265afdff3eb0f1c","unresolved":false,"context_lines":[{"line_number":2447,"context_line":"    :raise ValueError: If traits is empty or None."},{"line_number":2448,"context_line":"    \"\"\""},{"line_number":2449,"context_line":"    if not traits:"},{"line_number":2450,"context_line":"        raise ValueError(\u0027traits must not be empty\u0027)"},{"line_number":2451,"context_line":""},{"line_number":2452,"context_line":"    rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rpt\")"},{"line_number":2453,"context_line":"    sel \u003d sa.select([rptt.c.resource_provider_id])"}],"source_content_type":"text/x-python","patch_set":12,"id":"1f485f77_aac96759","line":2450,"range":{"start_line":2450,"start_character":25,"end_line":2450,"end_character":51},"in_reply_to":"1f485f77_cbc83223","updated":"2017-11-17 12:25:04.000000000","message":"Interestingly, if we hit this, it is surely a developer error, so we would want it to be in English.  But I guess in such cases we find the line by stack trace anyway.\n\nWill fix here if another patch set happens; else in a followon.","commit_id":"28e0d283c5f42f252dbda3c5c093f05e9fab44ab"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3bc5302143021b56db46a0a6e817b153753afb86","unresolved":false,"context_lines":[{"line_number":2903,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2904,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":2905,"context_line":"            if still_missing_traits:"},{"line_number":2906,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: missing traits \u0027"},{"line_number":2907,"context_line":"                          \u0027are not satisfied even by sharing providers: (%s).\u0027,"},{"line_number":2908,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(still_missing_traits))"},{"line_number":2909,"context_line":"                continue"},{"line_number":2910,"context_line":""},{"line_number":2911,"context_line":"            resource_requests \u003d ns_res_requests + list(shared_res_requests)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1f485f77_e21f7669","line":2908,"range":{"start_line":2906,"start_character":0,"end_line":2908,"end_character":70},"updated":"2017-11-20 00:27:13.000000000","message":"This is not excluding non-sharing provider itself. This is excluding a specific combination of a non-sharing provider and sharing providers. So, here, why don\u0027t we add sharing providers info, shared_rp_id(s), for debugging?","commit_id":"28e0d283c5f42f252dbda3c5c093f05e9fab44ab"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3bc5302143021b56db46a0a6e817b153753afb86","unresolved":false,"context_lines":[{"line_number":2926,"context_line":"    # Look up the internal ID for each identified rp UUID"},{"line_number":2927,"context_line":"    alloc_req_rp_ids \u003d set()"},{"line_number":2928,"context_line":"    for rp_uuid in alloc_req_rp_uuids:"},{"line_number":2929,"context_line":"        for rp_id, summary in summaries.items():"},{"line_number":2930,"context_line":"            if summary.resource_provider.uuid \u003d\u003d rp_uuid:"},{"line_number":2931,"context_line":"                alloc_req_rp_ids.add(rp_id)"},{"line_number":2932,"context_line":""},{"line_number":2933,"context_line":"    p_sums_ids \u003d set(summaries)"},{"line_number":2934,"context_line":"    eliminated_rp_ids \u003d p_sums_ids - alloc_req_rp_ids"}],"source_content_type":"text/x-python","patch_set":12,"id":"1f485f77_00904549","line":2931,"range":{"start_line":2929,"start_character":8,"end_line":2931,"end_character":43},"updated":"2017-11-20 00:27:13.000000000","message":"nit: this is the second time to translate rp_uuid-\u003erp_id... It is probably good to add a function later in the series.","commit_id":"28e0d283c5f42f252dbda3c5c093f05e9fab44ab"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d433f7299e269d04a2dd4a9bce3bf17d869818d5","unresolved":false,"context_lines":[{"line_number":2835,"context_line":"        # provider(s)"},{"line_number":2836,"context_line":"        need_traits \u003d set(required_traits)"},{"line_number":2837,"context_line":"        missing_traits \u003d set()"},{"line_number":2838,"context_line":"        if required_traits:"},{"line_number":2839,"context_line":"            ns_traits \u003d set(prov_traits.get(ns_rp_id, []))"},{"line_number":2840,"context_line":"            missing_traits \u003d need_traits - ns_traits"},{"line_number":2841,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_b870ea09","line":2838,"updated":"2017-11-27 15:56:34.000000000","message":"I thin we don\u0027t need this conditional\nWhen required_traits is empty then missing_traits will be always empty.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3b6966a3c219da50b57ff5e26e14c50ac600326","unresolved":false,"context_lines":[{"line_number":2835,"context_line":"        # provider(s)"},{"line_number":2836,"context_line":"        need_traits \u003d set(required_traits)"},{"line_number":2837,"context_line":"        missing_traits \u003d set()"},{"line_number":2838,"context_line":"        if required_traits:"},{"line_number":2839,"context_line":"            ns_traits \u003d set(prov_traits.get(ns_rp_id, []))"},{"line_number":2840,"context_line":"            missing_traits \u003d need_traits - ns_traits"},{"line_number":2841,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"df87a7cf_0466f8ee","line":2838,"in_reply_to":"ff82abbf_934dae3f","updated":"2017-12-01 14:43:45.000000000","message":"ok, will change.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a58b2da185764c0692dddec59ce130ff24aa5578","unresolved":false,"context_lines":[{"line_number":2835,"context_line":"        # provider(s)"},{"line_number":2836,"context_line":"        need_traits \u003d set(required_traits)"},{"line_number":2837,"context_line":"        missing_traits \u003d set()"},{"line_number":2838,"context_line":"        if required_traits:"},{"line_number":2839,"context_line":"            ns_traits \u003d set(prov_traits.get(ns_rp_id, []))"},{"line_number":2840,"context_line":"            missing_traits \u003d need_traits - ns_traits"},{"line_number":2841,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_c2329010","line":2838,"in_reply_to":"ff82abbf_b870ea09","updated":"2017-11-27 21:21:35.000000000","message":"So this whole block reduces to\n\n missing_traits \u003d set(required_traits) - set(prov_traits.get(ns_rp_id, []))\n\n?","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"863369867b6e8ae54d1c2d3c808fd44c3e549778","unresolved":false,"context_lines":[{"line_number":2835,"context_line":"        # provider(s)"},{"line_number":2836,"context_line":"        need_traits \u003d set(required_traits)"},{"line_number":2837,"context_line":"        missing_traits \u003d set()"},{"line_number":2838,"context_line":"        if required_traits:"},{"line_number":2839,"context_line":"            ns_traits \u003d set(prov_traits.get(ns_rp_id, []))"},{"line_number":2840,"context_line":"            missing_traits \u003d need_traits - ns_traits"},{"line_number":2841,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"df87a7cf_24ff7c68","line":2838,"in_reply_to":"ff82abbf_c2329010","updated":"2017-12-01 14:47:22.000000000","message":"Done","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"de2b23ac86a186a46e20c97c292a9a3525508313","unresolved":false,"context_lines":[{"line_number":2835,"context_line":"        # provider(s)"},{"line_number":2836,"context_line":"        need_traits \u003d set(required_traits)"},{"line_number":2837,"context_line":"        missing_traits \u003d set()"},{"line_number":2838,"context_line":"        if required_traits:"},{"line_number":2839,"context_line":"            ns_traits \u003d set(prov_traits.get(ns_rp_id, []))"},{"line_number":2840,"context_line":"            missing_traits \u003d need_traits - ns_traits"},{"line_number":2841,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_934dae3f","line":2838,"in_reply_to":"ff82abbf_c2329010","updated":"2017-11-28 11:24:11.000000000","message":"Yes, it does. I also tried it now and it passes the tests.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d433f7299e269d04a2dd4a9bce3bf17d869818d5","unresolved":false,"context_lines":[{"line_number":2895,"context_line":"            for shared_res_req in shared_res_requests:"},{"line_number":2896,"context_line":"                sharing_rp_uuid \u003d shared_res_req.resource_provider.uuid"},{"line_number":2897,"context_line":"                shared_rp_id \u003d None"},{"line_number":2898,"context_line":"                for rp_id, summary in summaries.items():"},{"line_number":2899,"context_line":"                    if summary.resource_provider.uuid \u003d\u003d sharing_rp_uuid:"},{"line_number":2900,"context_line":"                        shared_rp_id \u003d rp_id"},{"line_number":2901,"context_line":"                        break"},{"line_number":2902,"context_line":"                share_prov_traits \u003d prov_traits.get(shared_rp_id, [])"},{"line_number":2903,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2904,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_78c4a2f5","line":2901,"range":{"start_line":2898,"start_character":1,"end_line":2901,"end_character":29},"updated":"2017-11-27 15:56:34.000000000","message":"nit: I would move this to a separate helper to reduce nesting and therefore enhance readability.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c3b6966a3c219da50b57ff5e26e14c50ac600326","unresolved":false,"context_lines":[{"line_number":2895,"context_line":"            for shared_res_req in shared_res_requests:"},{"line_number":2896,"context_line":"                sharing_rp_uuid \u003d shared_res_req.resource_provider.uuid"},{"line_number":2897,"context_line":"                shared_rp_id \u003d None"},{"line_number":2898,"context_line":"                for rp_id, summary in summaries.items():"},{"line_number":2899,"context_line":"                    if summary.resource_provider.uuid \u003d\u003d sharing_rp_uuid:"},{"line_number":2900,"context_line":"                        shared_rp_id \u003d rp_id"},{"line_number":2901,"context_line":"                        break"},{"line_number":2902,"context_line":"                share_prov_traits \u003d prov_traits.get(shared_rp_id, [])"},{"line_number":2903,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":2904,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"}],"source_content_type":"text/x-python","patch_set":13,"id":"df87a7cf_04ab58d6","line":2901,"range":{"start_line":2898,"start_character":1,"end_line":2901,"end_character":29},"in_reply_to":"ff82abbf_78c4a2f5","updated":"2017-12-01 14:43:45.000000000","message":"I\u0027ll do that in a separate patch.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"edd11d9ecbcdfae71ab9076ddb313be2464684d1","unresolved":false,"context_lines":[{"line_number":2695,"context_line":"    \"\"\"Returns a list of resource provider internal IDs that have ANY of the"},{"line_number":2696,"context_line":"    supplied traits."},{"line_number":2697,"context_line":""},{"line_number":2698,"context_line":"    NOTE: Don\u0027t call this method with no required_traits."},{"line_number":2699,"context_line":""},{"line_number":2700,"context_line":"    :param ctx: Session context to use"},{"line_number":2701,"context_line":"    :param traits: A map, keyed by trait string name, of trait internal IDs, at"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_94257f0b","line":2698,"range":{"start_line":2698,"start_character":4,"end_line":2698,"end_character":57},"updated":"2017-12-13 09:45:21.000000000","message":"nit: I think the ValueError is enough warning below. I guess this NOTE was added at a time when we did not have the if at L2707.","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"014e681c81199e3c17f3c451f602f74ec97dab8f","unresolved":false,"context_lines":[{"line_number":2695,"context_line":"    \"\"\"Returns a list of resource provider internal IDs that have ANY of the"},{"line_number":2696,"context_line":"    supplied traits."},{"line_number":2697,"context_line":""},{"line_number":2698,"context_line":"    NOTE: Don\u0027t call this method with no required_traits."},{"line_number":2699,"context_line":""},{"line_number":2700,"context_line":"    :param ctx: Session context to use"},{"line_number":2701,"context_line":"    :param traits: A map, keyed by trait string name, of trait internal IDs, at"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_2ef9804c","line":2698,"range":{"start_line":2698,"start_character":4,"end_line":2698,"end_character":57},"in_reply_to":"df87a7cf_94257f0b","updated":"2017-12-13 15:53:01.000000000","message":"Done","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"8157058e06cba26b41c521f1b2f8f8efbcaf16f7","unresolved":false,"context_lines":[{"line_number":3104,"context_line":"        if has_all:"},{"line_number":3105,"context_line":"            # If this resource provider has all the requested resources, then"},{"line_number":3106,"context_line":"            # it must also have ALL required traits"},{"line_number":3107,"context_line":"            if missing_traits:"},{"line_number":3108,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: it has all \u0027"},{"line_number":3109,"context_line":"                          \u0027resources but is missing traits (%s).\u0027,"},{"line_number":3110,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(missing_traits))"},{"line_number":3111,"context_line":"                continue"},{"line_number":3112,"context_line":"            req \u003d _allocation_request_for_provider(ctx, requested_resources,"},{"line_number":3113,"context_line":"                                                   ns_rp_uuid)"},{"line_number":3114,"context_line":"            alloc_requests.append(req)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_a2be33d2","line":3111,"range":{"start_line":3107,"start_character":12,"end_line":3111,"end_character":24},"updated":"2017-12-12 03:21:16.000000000","message":"This can cause a bug that \"rps which satisfies all quantitative aspects gives up satisfying qualitative aspects without checking out sharing providers\". I haven\u0027t tested but my idea is to use \"if *not* missing_traits\" clause instead in order to fall them back to the following sequence to check up sharing resources.","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"edd11d9ecbcdfae71ab9076ddb313be2464684d1","unresolved":false,"context_lines":[{"line_number":3104,"context_line":"        if has_all:"},{"line_number":3105,"context_line":"            # If this resource provider has all the requested resources, then"},{"line_number":3106,"context_line":"            # it must also have ALL required traits"},{"line_number":3107,"context_line":"            if missing_traits:"},{"line_number":3108,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: it has all \u0027"},{"line_number":3109,"context_line":"                          \u0027resources but is missing traits (%s).\u0027,"},{"line_number":3110,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(missing_traits))"},{"line_number":3111,"context_line":"                continue"},{"line_number":3112,"context_line":"            req \u003d _allocation_request_for_provider(ctx, requested_resources,"},{"line_number":3113,"context_line":"                                                   ns_rp_uuid)"},{"line_number":3114,"context_line":"            alloc_requests.append(req)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_cf7c9bed","line":3111,"range":{"start_line":3107,"start_character":12,"end_line":3111,"end_character":24},"in_reply_to":"df87a7cf_7003febf","updated":"2017-12-13 09:45:21.000000000","message":"I always understood the comment above (L3105) as a rule that if the resources are coming from this RP then we want to have that the traits are also coming from this RP and not from sharing RPs. Anyhow it most probably only means that the comments above needs adjustments as well in the follow up changing this behavior.","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"67e47a787ddd99d60aa6104f5fdacd22c571b589","unresolved":false,"context_lines":[{"line_number":3104,"context_line":"        if has_all:"},{"line_number":3105,"context_line":"            # If this resource provider has all the requested resources, then"},{"line_number":3106,"context_line":"            # it must also have ALL required traits"},{"line_number":3107,"context_line":"            if missing_traits:"},{"line_number":3108,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: it has all \u0027"},{"line_number":3109,"context_line":"                          \u0027resources but is missing traits (%s).\u0027,"},{"line_number":3110,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(missing_traits))"},{"line_number":3111,"context_line":"                continue"},{"line_number":3112,"context_line":"            req \u003d _allocation_request_for_provider(ctx, requested_resources,"},{"line_number":3113,"context_line":"                                                   ns_rp_uuid)"},{"line_number":3114,"context_line":"            alloc_requests.append(req)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_7003febf","line":3111,"range":{"start_line":3107,"start_character":12,"end_line":3111,"end_character":24},"in_reply_to":"df87a7cf_95e47d3d","updated":"2017-12-13 00:24:46.000000000","message":"\u003e Would you mind if I just add a note in here?\n\nI don\u0027t mind.\nI agree that fixing it in a separate patch is better.","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a37605ae60fef4bae645bb87155ae0a0cdb84297","unresolved":false,"context_lines":[{"line_number":3104,"context_line":"        if has_all:"},{"line_number":3105,"context_line":"            # If this resource provider has all the requested resources, then"},{"line_number":3106,"context_line":"            # it must also have ALL required traits"},{"line_number":3107,"context_line":"            if missing_traits:"},{"line_number":3108,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: it has all \u0027"},{"line_number":3109,"context_line":"                          \u0027resources but is missing traits (%s).\u0027,"},{"line_number":3110,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(missing_traits))"},{"line_number":3111,"context_line":"                continue"},{"line_number":3112,"context_line":"            req \u003d _allocation_request_for_provider(ctx, requested_resources,"},{"line_number":3113,"context_line":"                                                   ns_rp_uuid)"},{"line_number":3114,"context_line":"            alloc_requests.append(req)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_95e47d3d","line":3111,"range":{"start_line":3107,"start_character":12,"end_line":3111,"end_character":24},"in_reply_to":"df87a7cf_a2be33d2","updated":"2017-12-12 15:43:41.000000000","message":"Good point. Would you mind if I just add a note in here, though, and fix that in a followup patch. I\u0027m afraid this patch will grow to an unreviewable size if I handle that scenario inline here...","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"014e681c81199e3c17f3c451f602f74ec97dab8f","unresolved":false,"context_lines":[{"line_number":3104,"context_line":"        if has_all:"},{"line_number":3105,"context_line":"            # If this resource provider has all the requested resources, then"},{"line_number":3106,"context_line":"            # it must also have ALL required traits"},{"line_number":3107,"context_line":"            if missing_traits:"},{"line_number":3108,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: it has all \u0027"},{"line_number":3109,"context_line":"                          \u0027resources but is missing traits (%s).\u0027,"},{"line_number":3110,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(missing_traits))"},{"line_number":3111,"context_line":"                continue"},{"line_number":3112,"context_line":"            req \u003d _allocation_request_for_provider(ctx, requested_resources,"},{"line_number":3113,"context_line":"                                                   ns_rp_uuid)"},{"line_number":3114,"context_line":"            alloc_requests.append(req)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_8ee8cc7c","line":3111,"range":{"start_line":3107,"start_character":12,"end_line":3111,"end_character":24},"in_reply_to":"df87a7cf_cf7c9bed","updated":"2017-12-13 15:53:01.000000000","message":"Done","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"8157058e06cba26b41c521f1b2f8f8efbcaf16f7","unresolved":false,"context_lines":[{"line_number":3157,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":3158,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":3159,"context_line":"            if still_missing_traits:"},{"line_number":3160,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: missing traits \u0027"},{"line_number":3161,"context_line":"                          \u0027are not satisfied even by sharing providers: (%s).\u0027,"},{"line_number":3162,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(still_missing_traits))"},{"line_number":3163,"context_line":"                continue"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":"            resource_requests \u003d ns_res_requests + list(shared_res_requests)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_62826b02","line":3162,"range":{"start_line":3160,"start_character":16,"end_line":3162,"end_character":70},"updated":"2017-12-12 03:21:16.000000000","message":"In order to avoid confusion, it is good to add non-sharing rps info. This is not excluding non-sharing provider itself, this is excluding a specific combination.","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a37605ae60fef4bae645bb87155ae0a0cdb84297","unresolved":false,"context_lines":[{"line_number":3157,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":3158,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":3159,"context_line":"            if still_missing_traits:"},{"line_number":3160,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: missing traits \u0027"},{"line_number":3161,"context_line":"                          \u0027are not satisfied even by sharing providers: (%s).\u0027,"},{"line_number":3162,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(still_missing_traits))"},{"line_number":3163,"context_line":"                continue"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":"            resource_requests \u003d ns_res_requests + list(shared_res_requests)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_b5b18146","line":3162,"range":{"start_line":3160,"start_character":16,"end_line":3162,"end_character":70},"in_reply_to":"df87a7cf_62826b02","updated":"2017-12-12 15:43:41.000000000","message":"Sorry, I\u0027m not quite following you, Tetsuro... what information about non-sharing rps would you like me to add to the above log message?","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"67e47a787ddd99d60aa6104f5fdacd22c571b589","unresolved":false,"context_lines":[{"line_number":3157,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":3158,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":3159,"context_line":"            if still_missing_traits:"},{"line_number":3160,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: missing traits \u0027"},{"line_number":3161,"context_line":"                          \u0027are not satisfied even by sharing providers: (%s).\u0027,"},{"line_number":3162,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(still_missing_traits))"},{"line_number":3163,"context_line":"                continue"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":"            resource_requests \u003d ns_res_requests + list(shared_res_requests)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_f0ce2e5a","line":3162,"range":{"start_line":3160,"start_character":16,"end_line":3162,"end_character":70},"in_reply_to":"df87a7cf_b5b18146","updated":"2017-12-13 00:24:46.000000000","message":"Sorry for not making it clear.\n\nI thought it can be confusing to log here just \"excluding non-sharing provider \u003crp_id\u003e\" because there is a possibility that we get that \u003crp_id\u003e at last in alloc_candidates with a different combination of \u003cshared_rp_ids\u003e in this iteration of itertools.product(*shared_request_groups).\n\nSo strictly speaking, what to log here is a kind of\n\"Excluding a combination of non-sharing provider \u003crp_id\u003e with \u003cshared_rp_ids\u003e: missing traits are \u003cstill missing traits\u003e\"\n\nHowever, this is nit. My -1 point was L3111 above.","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"014e681c81199e3c17f3c451f602f74ec97dab8f","unresolved":false,"context_lines":[{"line_number":3157,"context_line":"                sharing_traits |\u003d set(share_prov_traits)"},{"line_number":3158,"context_line":"            still_missing_traits \u003d missing_traits - sharing_traits"},{"line_number":3159,"context_line":"            if still_missing_traits:"},{"line_number":3160,"context_line":"                LOG.debug(\u0027Excluding non-sharing provider %s: missing traits \u0027"},{"line_number":3161,"context_line":"                          \u0027are not satisfied even by sharing providers: (%s).\u0027,"},{"line_number":3162,"context_line":"                          ns_rp_uuid, \u0027, \u0027.join(still_missing_traits))"},{"line_number":3163,"context_line":"                continue"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":"            resource_requests \u003d ns_res_requests + list(shared_res_requests)"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_eef39872","line":3162,"range":{"start_line":3160,"start_character":16,"end_line":3162,"end_character":70},"in_reply_to":"df87a7cf_f0ce2e5a","updated":"2017-12-13 15:53:01.000000000","message":"Done","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"}],"nova/tests/functional/db/test_allocation_candidates.py":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"3bc5302143021b56db46a0a6e817b153753afb86","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"            [(\u0027four\u0027, fields.ResourceClass.VCPU, 1),"},{"line_number":1073,"context_line":"             (\u0027four\u0027, fields.ResourceClass.MEMORY_MB, 64),"},{"line_number":1074,"context_line":"             (\u0027ss4\u0027, fields.ResourceClass.DISK_GB, 1500)],"},{"line_number":1075,"context_line":"            [(\u0027five\u0027, fields.ResourceClass.VCPU, 1),"},{"line_number":1076,"context_line":"             (\u0027five\u0027, fields.ResourceClass.MEMORY_MB, 64),"},{"line_number":1077,"context_line":"             (\u0027ss5\u0027, fields.ResourceClass.DISK_GB, 1500)],"},{"line_number":1078,"context_line":"            [(\u0027six\u0027, fields.ResourceClass.VCPU, 1),"},{"line_number":1079,"context_line":"             (\u0027six\u0027, fields.ResourceClass.MEMORY_MB, 64),"},{"line_number":1080,"context_line":"             (\u0027ss6\u0027, fields.ResourceClass.DISK_GB, 1500)],"}],"source_content_type":"text/x-python","patch_set":12,"id":"1f485f77_a0d29126","line":1077,"range":{"start_line":1075,"start_character":12,"end_line":1077,"end_character":58},"updated":"2017-11-20 00:27:13.000000000","message":"This case is...\n\n cn5: no traits\n ss5: HW_CPU_X86_AVX+CUSTOM_FOO\n\nIdeally, don\u0027t we want the trait HW_CPU_X86_AVX to be on compute nodes? The same can be said in the case of cn13-ss13_2. Possibly it is only that we don\u0027t set CPU traits on a RP for shared storage in the actual usecase?","commit_id":"28e0d283c5f42f252dbda3c5c093f05e9fab44ab"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d433f7299e269d04a2dd4a9bce3bf17d869818d5","unresolved":false,"context_lines":[{"line_number":896,"context_line":"        self.assertEqual(1, len(p_sums))"},{"line_number":897,"context_line":"        expected_prov_uuids \u003d set([cn3.uuid])"},{"line_number":898,"context_line":"        self.assertEqual(expected_prov_uuids,"},{"line_number":899,"context_line":"                         _provider_uuids_from_iterable(p_sums))"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    def test_multiple_aggregates_with_traits(self):"},{"line_number":902,"context_line":"        # Non-sharing RP has all requested resources, no traits, no aggregate."}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_6b8a8a6a","line":899,"updated":"2017-11-27 15:56:34.000000000","message":"I would assert also that we only get a single allocation candidate and that is for cn3","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d433f7299e269d04a2dd4a9bce3bf17d869818d5","unresolved":false,"context_lines":[{"line_number":1087,"context_line":"        # TODO(efried): This is what we should be getting:"},{"line_number":1088,"context_line":"        expected \u003d ["},{"line_number":1089,"context_line":"            # RP one should never show up because no traits."},{"line_number":1090,"context_line":"            # Agg 2 should never show up because the traits are only on the"},{"line_number":1091,"context_line":"            # sharing RP, but the sharing RP doesn\u0027t provide any resources."},{"line_number":1092,"context_line":"            # Agg 3 similarly"},{"line_number":1093,"context_line":"            # Aggs 4, 5, 6, 7, and 8 always gets disk from the sharing RP,"},{"line_number":1094,"context_line":"            # because that\u0027s the only way all resources and traits"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_4bd46620","line":1091,"range":{"start_line":1090,"start_character":0,"end_line":1091,"end_character":75},"updated":"2017-11-27 15:56:34.000000000","message":"Is it a rule to avoid getting an SSD trait for a compute RP and getting the actual disk resource from a sharing RP?","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a58b2da185764c0692dddec59ce130ff24aa5578","unresolved":false,"context_lines":[{"line_number":1087,"context_line":"        # TODO(efried): This is what we should be getting:"},{"line_number":1088,"context_line":"        expected \u003d ["},{"line_number":1089,"context_line":"            # RP one should never show up because no traits."},{"line_number":1090,"context_line":"            # Agg 2 should never show up because the traits are only on the"},{"line_number":1091,"context_line":"            # sharing RP, but the sharing RP doesn\u0027t provide any resources."},{"line_number":1092,"context_line":"            # Agg 3 similarly"},{"line_number":1093,"context_line":"            # Aggs 4, 5, 6, 7, and 8 always gets disk from the sharing RP,"},{"line_number":1094,"context_line":"            # because that\u0027s the only way all resources and traits"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_855492ff","line":1091,"range":{"start_line":1090,"start_character":0,"end_line":1091,"end_character":75},"in_reply_to":"ff82abbf_4bd46620","updated":"2017-11-27 21:21:35.000000000","message":"We don\u0027t have the ability to express that kind of rule at all.  That\u0027s what https://bugs.launchpad.net/nova/+bug/1724633 is about.\n\nFor right now, we seem to have a rule that sharing providers *must* provide resources in order to be included in the results; but non-sharing providers don\u0027t need to do so.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"de2b23ac86a186a46e20c97c292a9a3525508313","unresolved":false,"context_lines":[{"line_number":1087,"context_line":"        # TODO(efried): This is what we should be getting:"},{"line_number":1088,"context_line":"        expected \u003d ["},{"line_number":1089,"context_line":"            # RP one should never show up because no traits."},{"line_number":1090,"context_line":"            # Agg 2 should never show up because the traits are only on the"},{"line_number":1091,"context_line":"            # sharing RP, but the sharing RP doesn\u0027t provide any resources."},{"line_number":1092,"context_line":"            # Agg 3 similarly"},{"line_number":1093,"context_line":"            # Aggs 4, 5, 6, 7, and 8 always gets disk from the sharing RP,"},{"line_number":1094,"context_line":"            # because that\u0027s the only way all resources and traits"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_73fcaab4","line":1091,"range":{"start_line":1090,"start_character":0,"end_line":1091,"end_character":75},"in_reply_to":"ff82abbf_855492ff","updated":"2017-11-28 11:24:11.000000000","message":"It is a strange rule, but I guess we will think about it as part of all the open bugs regarding sharing RPs.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d433f7299e269d04a2dd4a9bce3bf17d869818d5","unresolved":false,"context_lines":[{"line_number":1148,"context_line":"             (\u0027ss15_2\u0027, fields.ResourceClass.DISK_GB, 1500)],"},{"line_number":1149,"context_line":"        ]"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"        # TODO(efried): This is what we\u0027re getting instead:"},{"line_number":1152,"context_line":"        expected \u003d ["},{"line_number":1153,"context_line":"            # These two are legit"},{"line_number":1154,"context_line":"            [(\u0027nine\u0027, fields.ResourceClass.DISK_GB, 1500),"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_eba0da92","line":1151,"updated":"2017-11-27 15:56:34.000000000","message":"Do we have a bug number to refer here?","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"de2b23ac86a186a46e20c97c292a9a3525508313","unresolved":false,"context_lines":[{"line_number":1148,"context_line":"             (\u0027ss15_2\u0027, fields.ResourceClass.DISK_GB, 1500)],"},{"line_number":1149,"context_line":"        ]"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"        # TODO(efried): This is what we\u0027re getting instead:"},{"line_number":1152,"context_line":"        expected \u003d ["},{"line_number":1153,"context_line":"            # These two are legit"},{"line_number":1154,"context_line":"            [(\u0027nine\u0027, fields.ResourceClass.DISK_GB, 1500),"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_9324ce5f","line":1151,"in_reply_to":"ff82abbf_c577fa9f","updated":"2017-11-28 11:24:11.000000000","message":"Which patch does split this up? As I see https://review.openstack.org/#/c/518633/15/nova/tests/functional/db/test_allocation_candidates.py does not do the splitting of this.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a58b2da185764c0692dddec59ce130ff24aa5578","unresolved":false,"context_lines":[{"line_number":1148,"context_line":"             (\u0027ss15_2\u0027, fields.ResourceClass.DISK_GB, 1500)],"},{"line_number":1149,"context_line":"        ]"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"        # TODO(efried): This is what we\u0027re getting instead:"},{"line_number":1152,"context_line":"        expected \u003d ["},{"line_number":1153,"context_line":"            # These two are legit"},{"line_number":1154,"context_line":"            [(\u0027nine\u0027, fields.ResourceClass.DISK_GB, 1500),"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_c577fa9f","line":1151,"in_reply_to":"ff82abbf_eba0da92","updated":"2017-11-27 21:21:35.000000000","message":"There\u0027s a bunch of bugs manifested here.  I believe they\u0027re split out and marked with bug numbers later in the series.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7cac09fbf8c2c3f1b0c137812e827d2a61edfcfa","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        cn1.set_traits([avx2_t])"},{"line_number":659,"context_line":"        cn2.set_traits([avx2_t])"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        alloc_cands \u003d self._get_allocation_candidates(["},{"line_number":662,"context_line":"            placement_lib.RequestGroup("},{"line_number":663,"context_line":"                use_same_provider\u003dFalse,"},{"line_number":664,"context_line":"                resources\u003dself.requested_resources,"},{"line_number":665,"context_line":"                required_traits\u003dset([os_traits.HW_CPU_X86_AVX2]),"},{"line_number":666,"context_line":"            )],"},{"line_number":667,"context_line":"        )"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        p_sums \u003d alloc_cands.provider_summaries"},{"line_number":670,"context_line":"        # There should be 2 compute node providers and 1 shared storage"}],"source_content_type":"text/x-python","patch_set":15,"id":"df87a7cf_d618c276","line":667,"range":{"start_line":661,"start_character":8,"end_line":667,"end_character":9},"updated":"2017-12-01 17:54:22.000000000","message":"Since this test is asserting that the same query returns different results before and after associating the trait, it would be cleaner to make this a small function, and call it from both places. This assures that over time the two copies won\u0027t drift due to bitrot. It also makes reviewing easier, as we don\u0027t have to micro-examine to make sure these two calls are identical.\n\nReading down further, there are several other calls that are identical to this, with the exception of the required traits being passed in. So maybe it should be a private method of the class.","commit_id":"1e75c5bdfcf124118caf7cce6b3932927e301552"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cbf67cef0cbd5c71ffcb1c49c0f2092bd681d005","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        cn1.set_traits([avx2_t])"},{"line_number":659,"context_line":"        cn2.set_traits([avx2_t])"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        alloc_cands \u003d self._get_allocation_candidates(["},{"line_number":662,"context_line":"            placement_lib.RequestGroup("},{"line_number":663,"context_line":"                use_same_provider\u003dFalse,"},{"line_number":664,"context_line":"                resources\u003dself.requested_resources,"},{"line_number":665,"context_line":"                required_traits\u003dset([os_traits.HW_CPU_X86_AVX2]),"},{"line_number":666,"context_line":"            )],"},{"line_number":667,"context_line":"        )"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        p_sums \u003d alloc_cands.provider_summaries"},{"line_number":670,"context_line":"        # There should be 2 compute node providers and 1 shared storage"}],"source_content_type":"text/x-python","patch_set":15,"id":"df87a7cf_037cd7e9","line":667,"range":{"start_line":661,"start_character":8,"end_line":667,"end_character":9},"in_reply_to":"df87a7cf_39cd5a5d","updated":"2017-12-11 14:58:52.000000000","message":"agree with Eric that this should be reworked in a later patch in the series.","commit_id":"1e75c5bdfcf124118caf7cce6b3932927e301552"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a833370f192a4c981833cd14b25f61df391ac1c9","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        cn1.set_traits([avx2_t])"},{"line_number":659,"context_line":"        cn2.set_traits([avx2_t])"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        alloc_cands \u003d self._get_allocation_candidates(["},{"line_number":662,"context_line":"            placement_lib.RequestGroup("},{"line_number":663,"context_line":"                use_same_provider\u003dFalse,"},{"line_number":664,"context_line":"                resources\u003dself.requested_resources,"},{"line_number":665,"context_line":"                required_traits\u003dset([os_traits.HW_CPU_X86_AVX2]),"},{"line_number":666,"context_line":"            )],"},{"line_number":667,"context_line":"        )"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        p_sums \u003d alloc_cands.provider_summaries"},{"line_number":670,"context_line":"        # There should be 2 compute node providers and 1 shared storage"}],"source_content_type":"text/x-python","patch_set":15,"id":"df87a7cf_39cd5a5d","line":667,"range":{"start_line":661,"start_character":8,"end_line":667,"end_character":9},"in_reply_to":"df87a7cf_d618c276","updated":"2017-12-01 22:48:10.000000000","message":"Which part are you suggesting to factor out?\n\nIn any case, if something is to be done about this, it should happen after the refactor with the test helpers [1].  We can fold it into the \u0027accumulated nits\u0027 patch [2].\n\n[1] https://review.openstack.org/#/c/518982/\n[2] https://review.openstack.org/#/c/521189/","commit_id":"1e75c5bdfcf124118caf7cce6b3932927e301552"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7cac09fbf8c2c3f1b0c137812e827d2a61edfcfa","unresolved":false,"context_lines":[{"line_number":831,"context_line":"        for cn in (cn1, cn2, cn3):"},{"line_number":832,"context_line":"            _set_traits(cn, os_traits.HW_CPU_X86_AVX2)"},{"line_number":833,"context_line":""},{"line_number":834,"context_line":"        alloc_cands \u003d self._get_allocation_candidates(["},{"line_number":835,"context_line":"            placement_lib.RequestGroup("},{"line_number":836,"context_line":"                use_same_provider\u003dFalse,"},{"line_number":837,"context_line":"                resources\u003dself.requested_resources,"},{"line_number":838,"context_line":"                required_traits\u003dset([os_traits.HW_CPU_X86_AVX2]),"},{"line_number":839,"context_line":"            )],"},{"line_number":840,"context_line":"        )"},{"line_number":841,"context_line":""},{"line_number":842,"context_line":"        p_sums \u003d alloc_cands.provider_summaries"},{"line_number":843,"context_line":"        # There should be 3 compute node providers and 1 shared storage"}],"source_content_type":"text/x-python","patch_set":15,"id":"df87a7cf_9651da11","line":840,"range":{"start_line":834,"start_character":0,"end_line":840,"end_character":9},"updated":"2017-12-01 17:54:22.000000000","message":"Same comment as above.","commit_id":"1e75c5bdfcf124118caf7cce6b3932927e301552"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cbf67cef0cbd5c71ffcb1c49c0f2092bd681d005","unresolved":false,"context_lines":[{"line_number":898,"context_line":"        self.assertEqual(expected_prov_uuids,"},{"line_number":899,"context_line":"                         _provider_uuids_from_iterable(p_sums))"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    def test_multiple_aggregates_with_traits(self):"},{"line_number":902,"context_line":"        # Non-sharing RP has all requested resources, no traits, no aggregate."},{"line_number":903,"context_line":"        cn1 \u003d self._create_provider(\u0027one\u0027)"},{"line_number":904,"context_line":"        _add_inventory(cn1, fields.ResourceClass.VCPU, 24)"}],"source_content_type":"text/x-python","patch_set":15,"id":"df87a7cf_e30f7385","line":901,"updated":"2017-12-11 14:58:52.000000000","message":"I\u0027ve removed this entire test method, since (as I complained in a previous couple review comments) it\u0027s too big and amorphous to include in this patch.\n\nI will re-add it in pieces later on.","commit_id":"1e75c5bdfcf124118caf7cce6b3932927e301552"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7cac09fbf8c2c3f1b0c137812e827d2a61edfcfa","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        # TODO(efried): This is what we should be getting:"},{"line_number":1088,"context_line":"        expected \u003d ["},{"line_number":1089,"context_line":"            # RP one should never show up because no traits."},{"line_number":1090,"context_line":"            # Agg 2 should never show up because the traits are only on the"},{"line_number":1091,"context_line":"            # sharing RP, but the sharing RP doesn\u0027t provide any resources."},{"line_number":1092,"context_line":"            # Agg 3 similarly"}],"source_content_type":"text/x-python","patch_set":15,"id":"df87a7cf_769a16c6","line":1089,"range":{"start_line":1089,"start_character":17,"end_line":1089,"end_character":20},"updated":"2017-12-01 17:54:22.000000000","message":"s/one/1  for consistency","commit_id":"1e75c5bdfcf124118caf7cce6b3932927e301552"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a833370f192a4c981833cd14b25f61df391ac1c9","unresolved":false,"context_lines":[{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        # TODO(efried): This is what we should be getting:"},{"line_number":1088,"context_line":"        expected \u003d ["},{"line_number":1089,"context_line":"            # RP one should never show up because no traits."},{"line_number":1090,"context_line":"            # Agg 2 should never show up because the traits are only on the"},{"line_number":1091,"context_line":"            # sharing RP, but the sharing RP doesn\u0027t provide any resources."},{"line_number":1092,"context_line":"            # Agg 3 similarly"}],"source_content_type":"text/x-python","patch_set":15,"id":"df87a7cf_59837e05","line":1089,"range":{"start_line":1089,"start_character":17,"end_line":1089,"end_character":20},"in_reply_to":"df87a7cf_769a16c6","updated":"2017-12-01 22:48:10.000000000","message":"Actually the RPs are named \u0027one\u0027, \u0027two\u0027, etc.  It\u0027s the aggregates that are numbered.","commit_id":"1e75c5bdfcf124118caf7cce6b3932927e301552"}],"nova/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3f4b6375_c584cdd5","updated":"2017-11-02 12:55:23.000000000","message":"Yeah, way more test permutations needed here.  In addition to the ones suggested in the previous patch (which should be duplicated in some fashion for the sharing cases), we need to cover things like:\n\n- Required trait on the sharing RP but none on the compute node (Alex and I believe this is busted with the current code).\n- Opposite of the above: compute node has some resources and all the required traits; and one or more sharing RP(s) provide(s) the remaining resources but none of the traits.\n- Variants with multiple sharing RPs.  E.g. one has the resource but not the traits; the other vice versa.\n- Request for resources and traits that are *only* satisfied by the sharing RP(s).  This should be allowed.\n- Resources are all satisfied by the sharing RPs, but the full set of traits is only satisfied when the compute node is included in the mix.  (I don\u0027t even know what we expect to happen here.)\n- Opposite of the above: resources are satisfied by the compute node (and a subset of sharing RPs) but another sharing RP would have to be included (providing no resources) to satisfy the full set of traits.  (Pretty sure this one should *not* work.)","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1f485f77_6ec63e4b","in_reply_to":"3f4b6375_c584cdd5","updated":"2017-11-14 21:08:04.000000000","message":"Done in subsequent patches.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":3302,"context_line":"                         sorted([set([cn1.uuid, ss.uuid]),"},{"line_number":3303,"context_line":"                                 set([cn2.uuid, ss.uuid]), set([cn3.uuid])]))"},{"line_number":3304,"context_line":""},{"line_number":3305,"context_line":"        # Now we\u0027re going to add a set of required traits into the request mix."},{"line_number":3306,"context_line":"        # To start off, let\u0027s request a required trait that we know has not"},{"line_number":3307,"context_line":"        # been associated yet with any provider, and ensure we get no results"},{"line_number":3308,"context_line":"        p_alts \u003d rp_obj.AllocationCandidates.get_by_filters("}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_05342592","line":3305,"updated":"2017-11-02 12:55:23.000000000","message":"This seems to be largely a duplication of the addition above.  Do we need both?  (This one is a little more extensive, so keep this one.)","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":3302,"context_line":"                         sorted([set([cn1.uuid, ss.uuid]),"},{"line_number":3303,"context_line":"                                 set([cn2.uuid, ss.uuid]), set([cn3.uuid])]))"},{"line_number":3304,"context_line":""},{"line_number":3305,"context_line":"        # Now we\u0027re going to add a set of required traits into the request mix."},{"line_number":3306,"context_line":"        # To start off, let\u0027s request a required trait that we know has not"},{"line_number":3307,"context_line":"        # been associated yet with any provider, and ensure we get no results"},{"line_number":3308,"context_line":"        p_alts \u003d rp_obj.AllocationCandidates.get_by_filters("}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_2ede0611","line":3305,"in_reply_to":"3f4b6375_05342592","updated":"2017-11-14 21:08:04.000000000","message":"Meh, keeping both.","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":3321,"context_line":"        self.assertEqual(0, len(a_reqs))"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        # Now, if we then associate the required trait with all of our compute"},{"line_number":3324,"context_line":"        # nodes, we should get back both compute nodes since they both now"},{"line_number":3325,"context_line":"        # satisfy the required traits as well as the resource request"},{"line_number":3326,"context_line":"        avx2_t \u003d rp_obj.Trait.get_by_name(self.ctx, os_traits.HW_CPU_X86_AVX2)"},{"line_number":3327,"context_line":"        cn1.set_traits([avx2_t])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_050b05d6","line":3324,"range":{"start_line":3324,"start_character":36,"end_line":3324,"end_character":40},"updated":"2017-11-02 12:55:23.000000000","message":"all","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3663303637142c123cb45ce7a8df02c8b6ff364c","unresolved":false,"context_lines":[{"line_number":3321,"context_line":"        self.assertEqual(0, len(a_reqs))"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        # Now, if we then associate the required trait with all of our compute"},{"line_number":3324,"context_line":"        # nodes, we should get back both compute nodes since they both now"},{"line_number":3325,"context_line":"        # satisfy the required traits as well as the resource request"},{"line_number":3326,"context_line":"        avx2_t \u003d rp_obj.Trait.get_by_name(self.ctx, os_traits.HW_CPU_X86_AVX2)"},{"line_number":3327,"context_line":"        cn1.set_traits([avx2_t])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_e50b49d2","line":3324,"range":{"start_line":3324,"start_character":66,"end_line":3324,"end_character":70},"updated":"2017-11-02 12:55:23.000000000","message":"all","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":3321,"context_line":"        self.assertEqual(0, len(a_reqs))"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        # Now, if we then associate the required trait with all of our compute"},{"line_number":3324,"context_line":"        # nodes, we should get back both compute nodes since they both now"},{"line_number":3325,"context_line":"        # satisfy the required traits as well as the resource request"},{"line_number":3326,"context_line":"        avx2_t \u003d rp_obj.Trait.get_by_name(self.ctx, os_traits.HW_CPU_X86_AVX2)"},{"line_number":3327,"context_line":"        cn1.set_traits([avx2_t])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_4ed6e2f3","line":3324,"range":{"start_line":3324,"start_character":36,"end_line":3324,"end_character":40},"in_reply_to":"3f4b6375_050b05d6","updated":"2017-11-14 21:08:04.000000000","message":"Done","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0fe94aeb8c923723e99c650f2d42bb39c0403690","unresolved":false,"context_lines":[{"line_number":3321,"context_line":"        self.assertEqual(0, len(a_reqs))"},{"line_number":3322,"context_line":""},{"line_number":3323,"context_line":"        # Now, if we then associate the required trait with all of our compute"},{"line_number":3324,"context_line":"        # nodes, we should get back both compute nodes since they both now"},{"line_number":3325,"context_line":"        # satisfy the required traits as well as the resource request"},{"line_number":3326,"context_line":"        avx2_t \u003d rp_obj.Trait.get_by_name(self.ctx, os_traits.HW_CPU_X86_AVX2)"},{"line_number":3327,"context_line":"        cn1.set_traits([avx2_t])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_ee00ee69","line":3324,"range":{"start_line":3324,"start_character":66,"end_line":3324,"end_character":70},"in_reply_to":"3f4b6375_e50b49d2","updated":"2017-11-14 21:08:04.000000000","message":"Done","commit_id":"bf7bbd4efbdb40675801412fb5104ce218e9eeb3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d433f7299e269d04a2dd4a9bce3bf17d869818d5","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.api_db \u003d self.useFixture(fixtures.Database(database\u003d\u0027api\u0027))"},{"line_number":54,"context_line":"        self.ctx \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":55,"context_line":"        # For debugging purposes, helps us log AllocationRequestResource"},{"line_number":56,"context_line":"        self.rp_uuid_to_name \u003d {}"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _make_allocation(self, rp_uuid\u003dNone, inv_dict\u003dNone):"},{"line_number":59,"context_line":"        rp_uuid \u003d rp_uuid or uuidsentinel.allocation_resource_provider"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_ebf77a8a","line":56,"updated":"2017-11-27 15:56:34.000000000","message":"I don\u0027t find the usage of this below.","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a58b2da185764c0692dddec59ce130ff24aa5578","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.api_db \u003d self.useFixture(fixtures.Database(database\u003d\u0027api\u0027))"},{"line_number":54,"context_line":"        self.ctx \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":55,"context_line":"        # For debugging purposes, helps us log AllocationRequestResource"},{"line_number":56,"context_line":"        self.rp_uuid_to_name \u003d {}"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _make_allocation(self, rp_uuid\u003dNone, inv_dict\u003dNone):"},{"line_number":59,"context_line":"        rp_uuid \u003d rp_uuid or uuidsentinel.allocation_resource_provider"}],"source_content_type":"text/x-python","patch_set":13,"id":"ff82abbf_45b0ca43","line":56,"in_reply_to":"ff82abbf_ebf77a8a","updated":"2017-11-27 21:21:35.000000000","message":"You\u0027re right - this is a leftover from the various refactors that split out test_allocation_candidates.\n\nhttps://review.openstack.org/#/c/517119/13/nova/tests/functional/db/test_allocation_candidates.py@94","commit_id":"8f5ab93ff5437dc226c464b65592dde1d5e861b9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"30837d68736c68ba880b0d2e9de66522a479260f","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from nova.tests import fixtures"},{"line_number":26,"context_line":"from nova.tests import uuidsentinel"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"DISK_INVENTORY \u003d dict("},{"line_number":30,"context_line":"    total\u003d200,"},{"line_number":31,"context_line":"    reserved\u003d10,"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_b28af760","line":28,"updated":"2017-12-12 15:14:45.000000000","message":"Random whitespace","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"014e681c81199e3c17f3c451f602f74ec97dab8f","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from nova.tests import fixtures"},{"line_number":26,"context_line":"from nova.tests import uuidsentinel"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"DISK_INVENTORY \u003d dict("},{"line_number":30,"context_line":"    total\u003d200,"},{"line_number":31,"context_line":"    reserved\u003d10,"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_ee98781c","line":28,"in_reply_to":"df87a7cf_b28af760","updated":"2017-12-13 15:53:01.000000000","message":"Done","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"edd11d9ecbcdfae71ab9076ddb313be2464684d1","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.useFixture(fixtures.Database())"},{"line_number":53,"context_line":"        self.api_db \u003d self.useFixture(fixtures.Database(database\u003d\u0027api\u0027))"},{"line_number":54,"context_line":"        self.ctx \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":55,"context_line":"        # For debugging purposes, helps us log AllocationRequestResource"},{"line_number":56,"context_line":"        self.rp_uuid_to_name \u003d {}"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _make_allocation(self, rp_uuid\u003dNone, inv_dict\u003dNone):"},{"line_number":59,"context_line":"        rp_uuid \u003d rp_uuid or uuidsentinel.allocation_resource_provider"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_32c49092","line":56,"range":{"start_line":55,"start_character":0,"end_line":56,"end_character":33},"updated":"2017-12-13 09:45:21.000000000","message":"This seems to be unused.","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"014e681c81199e3c17f3c451f602f74ec97dab8f","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self.useFixture(fixtures.Database())"},{"line_number":53,"context_line":"        self.api_db \u003d self.useFixture(fixtures.Database(database\u003d\u0027api\u0027))"},{"line_number":54,"context_line":"        self.ctx \u003d context.RequestContext(\u0027fake-user\u0027, \u0027fake-project\u0027)"},{"line_number":55,"context_line":"        # For debugging purposes, helps us log AllocationRequestResource"},{"line_number":56,"context_line":"        self.rp_uuid_to_name \u003d {}"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _make_allocation(self, rp_uuid\u003dNone, inv_dict\u003dNone):"},{"line_number":59,"context_line":"        rp_uuid \u003d rp_uuid or uuidsentinel.allocation_resource_provider"}],"source_content_type":"text/x-python","patch_set":17,"id":"df87a7cf_ce933400","line":56,"range":{"start_line":55,"start_character":0,"end_line":56,"end_character":33},"in_reply_to":"df87a7cf_32c49092","updated":"2017-12-13 15:53:01.000000000","message":"Done","commit_id":"98d47615252677ff5ebd05248f763e8997a1af9e"}]}
