)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Consider forbidden traits in early exit of _get_by_one_request"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Current, if there aren\u0027t any providers that have any of the"},{"line_number":10,"context_line":"required traits, just exit early in _get_by_one_request. But"},{"line_number":11,"context_line":"we should also consider the forbidden traits to optimize this"},{"line_number":12,"context_line":"attempt at a quick return."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_bf62ce3b","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":7},"updated":"2018-07-20 15:30:48.000000000","message":"currently","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"}],"nova/api/openstack/placement/objects/resource_provider.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"08e716a3d336a71b49fd1502ff5ab809281974be","unresolved":false,"context_lines":[{"line_number":2678,"context_line":""},{"line_number":2679,"context_line":""},{"line_number":2680,"context_line":"@db_api.placement_context_manager.reader"},{"line_number":2681,"context_line":"def _get_provider_ids_having_any_trait(ctx, traits, forbidden_traits\u003dNone):"},{"line_number":2682,"context_line":"    \"\"\"Returns a list of resource provider internal IDs that have ANY of the"},{"line_number":2683,"context_line":"    supplied traits and do NOT have ANY of the supplied forbidden_traits."},{"line_number":2684,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_141de075","line":2681,"updated":"2018-07-20 20:58:15.000000000","message":"I really prefer functions that do one thing and do that thing well and efficiently. Adding more parameters to this function is changing its purpose and making the interfaces messier.","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"8d0b7cfd947552a4eb75658dacd0d49183523506","unresolved":false,"context_lines":[{"line_number":2678,"context_line":""},{"line_number":2679,"context_line":""},{"line_number":2680,"context_line":"@db_api.placement_context_manager.reader"},{"line_number":2681,"context_line":"def _get_provider_ids_having_any_trait(ctx, traits, forbidden_traits\u003dNone):"},{"line_number":2682,"context_line":"    \"\"\"Returns a list of resource provider internal IDs that have ANY of the"},{"line_number":2683,"context_line":"    supplied traits and do NOT have ANY of the supplied forbidden_traits."},{"line_number":2684,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_0418401d","line":2681,"in_reply_to":"5f7c97a3_141de075","updated":"2018-07-23 01:59:01.000000000","message":"OK, thanks for your explanation. :)","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c6047a4345d19429b74a525b4e35766c3ed8e9f1","unresolved":false,"context_lines":[{"line_number":2691,"context_line":"                             have associated with it"},{"line_number":2692,"context_line":"    :raise ValueError: If traits is empty or None."},{"line_number":2693,"context_line":"    \"\"\""},{"line_number":2694,"context_line":"    if not traits:"},{"line_number":2695,"context_line":"        raise ValueError(_(\u0027traits must not be empty\u0027))"},{"line_number":2696,"context_line":""},{"line_number":2697,"context_line":"    rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rpt\")"},{"line_number":2698,"context_line":"    sel \u003d sa.select([rptt.c.resource_provider_id])"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_aeb13947","line":2695,"range":{"start_line":2694,"start_character":1,"end_line":2695,"end_character":55},"updated":"2018-07-20 18:47:23.000000000","message":"You\u0027re right, you would have to remove this...","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c6047a4345d19429b74a525b4e35766c3ed8e9f1","unresolved":false,"context_lines":[{"line_number":2696,"context_line":""},{"line_number":2697,"context_line":"    rptt \u003d sa.alias(_RP_TRAIT_TBL, name\u003d\"rpt\")"},{"line_number":2698,"context_line":"    sel \u003d sa.select([rptt.c.resource_provider_id])"},{"line_number":2699,"context_line":"    where_conds \u003d [rptt.c.trait_id.in_(traits.values())]"},{"line_number":2700,"context_line":"    if forbidden_traits:"},{"line_number":2701,"context_line":"        # get all providers which have forbid traits"},{"line_number":2702,"context_line":"        forbiden_rps \u003d _get_provider_ids_having_any_trait("}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_0ea70d0b","line":2699,"range":{"start_line":2699,"start_character":0,"end_line":2699,"end_character":56},"updated":"2018-07-20 18:47:23.000000000","message":"...and condition this WHERE clause on traits being nonempty...","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":2698,"context_line":"    sel \u003d sa.select([rptt.c.resource_provider_id])"},{"line_number":2699,"context_line":"    where_conds \u003d [rptt.c.trait_id.in_(traits.values())]"},{"line_number":2700,"context_line":"    if forbidden_traits:"},{"line_number":2701,"context_line":"        # get all providers which have forbid traits"},{"line_number":2702,"context_line":"        forbiden_rps \u003d _get_provider_ids_having_any_trait("},{"line_number":2703,"context_line":"            ctx, forbidden_traits)"},{"line_number":2704,"context_line":"        where_conds.append(~rptt.c.resource_provider_id.in_(forbiden_rps))"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_df6f8a26","line":2701,"range":{"start_line":2701,"start_character":39,"end_line":2701,"end_character":45},"updated":"2018-07-20 15:30:48.000000000","message":"forbidden","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":2701,"context_line":"        # get all providers which have forbid traits"},{"line_number":2702,"context_line":"        forbiden_rps \u003d _get_provider_ids_having_any_trait("},{"line_number":2703,"context_line":"            ctx, forbidden_traits)"},{"line_number":2704,"context_line":"        where_conds.append(~rptt.c.resource_provider_id.in_(forbiden_rps))"},{"line_number":2705,"context_line":"    sel \u003d sel.where(sa.and_(*where_conds))"},{"line_number":2706,"context_line":"    sel \u003d sel.group_by(rptt.c.resource_provider_id)"},{"line_number":2707,"context_line":"    return [r[0] for r in ctx.session.execute(sel)]"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_bfc72e11","line":2704,"updated":"2018-07-20 15:30:48.000000000","message":"# ...and exclude them from the results","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":2970,"context_line":""},{"line_number":2971,"context_line":""},{"line_number":2972,"context_line":"@db_api.placement_context_manager.reader"},{"line_number":2973,"context_line":"def _get_trees_with_traits(ctx, rp_ids, required_traits, forbidden_traits):"},{"line_number":2974,"context_line":"    \"\"\"Given a list of provider IDs, filter them to return a set of tuples of"},{"line_number":2975,"context_line":"    (provider ID, root provider ID) of providers which belong to a tree that"},{"line_number":2976,"context_line":"    can satisfy trait requirements."}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_9f0591c3","line":2973,"range":{"start_line":2973,"start_character":57,"end_line":2973,"end_character":73},"updated":"2018-07-20 15:30:48.000000000","message":"I *think* we can now get rid of this. The rp_ids we receive will already exclude any providers with any forbidden traits.","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":3953,"context_line":"            }"},{"line_number":3954,"context_line":""},{"line_number":3955,"context_line":"            # If there aren\u0027t any providers that have any of the"},{"line_number":3956,"context_line":"            # required traits and don\u0027t have any of forbidden traits,"},{"line_number":3957,"context_line":"            # just exit early..."},{"line_number":3958,"context_line":"            if required_trait_map:"},{"line_number":3959,"context_line":"                trait_rps \u003d _get_provider_ids_having_any_trait("}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_bff08e67","line":3956,"range":{"start_line":3956,"start_character":49,"end_line":3956,"end_character":51},"updated":"2018-07-20 15:30:48.000000000","message":"of the","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":3955,"context_line":"            # If there aren\u0027t any providers that have any of the"},{"line_number":3956,"context_line":"            # required traits and don\u0027t have any of forbidden traits,"},{"line_number":3957,"context_line":"            # just exit early..."},{"line_number":3958,"context_line":"            if required_trait_map:"},{"line_number":3959,"context_line":"                trait_rps \u003d _get_provider_ids_having_any_trait("},{"line_number":3960,"context_line":"                    context, required_trait_map, forbidden_trait_map)"},{"line_number":3961,"context_line":"                if not trait_rps:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_3fc55d90","line":3958,"range":{"start_line":3958,"start_character":15,"end_line":3958,"end_character":33},"updated":"2018-07-20 15:30:48.000000000","message":"This would be better as\n\n if required_trait_map or forbidden_trait_map:","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"34ad810e1704202bc1d0f871322533919528e684","unresolved":false,"context_lines":[{"line_number":3955,"context_line":"            # If there aren\u0027t any providers that have any of the"},{"line_number":3956,"context_line":"            # required traits and don\u0027t have any of forbidden traits,"},{"line_number":3957,"context_line":"            # just exit early..."},{"line_number":3958,"context_line":"            if required_trait_map:"},{"line_number":3959,"context_line":"                trait_rps \u003d _get_provider_ids_having_any_trait("},{"line_number":3960,"context_line":"                    context, required_trait_map, forbidden_trait_map)"},{"line_number":3961,"context_line":"                if not trait_rps:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_df430996","line":3958,"range":{"start_line":3958,"start_character":15,"end_line":3958,"end_character":33},"in_reply_to":"5f7c97a3_3fc55d90","updated":"2018-07-20 16:26:28.000000000","message":"I think you means the logic in PS1, but we can\u0027t do it, because we can\u0027t only get expected forbidden result from resouce_provider_traits table.\n\nFor example, \n1. RP1 have trait A (a trait + rp record in RPT_TAB)\n2. RP2 doesn\u0027t have any trait (no record in RPT_TAB)\n\nIf we pass a forbidden traitA, we will get back a [] in _get_provider_ids_having_any_trait, but expected is [rp2], because RP2 should meet the requirement.\n\nSo, this patch only solve the request that have traits(require) and forbidden traits(optional).\n\nFor only forbidden pass, I\u0027m not sure how to  only do forbidden trait in quick return, maybe we need check the count of RP has specific traits  equal to the count of all RP? If it\u0027s equal, then we return [](that means all rps have specific forbidden traits, we return [])? but it seems a little LOW effective, so, I\u0027m not sure should we add this logic in QUICK return. :)","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c6047a4345d19429b74a525b4e35766c3ed8e9f1","unresolved":false,"context_lines":[{"line_number":3955,"context_line":"            # If there aren\u0027t any providers that have any of the"},{"line_number":3956,"context_line":"            # required traits and don\u0027t have any of forbidden traits,"},{"line_number":3957,"context_line":"            # just exit early..."},{"line_number":3958,"context_line":"            if required_trait_map:"},{"line_number":3959,"context_line":"                trait_rps \u003d _get_provider_ids_having_any_trait("},{"line_number":3960,"context_line":"                    context, required_trait_map, forbidden_trait_map)"},{"line_number":3961,"context_line":"                if not trait_rps:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_ee94118e","line":3958,"range":{"start_line":3958,"start_character":15,"end_line":3958,"end_character":33},"in_reply_to":"5f7c97a3_df430996","updated":"2018-07-20 18:47:23.000000000","message":"...and then you could condition this on required and/or forbidden.","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"}],"nova/tests/functional/api/openstack/placement/db/test_resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"cde8e2026c0ce1d8a4ddff6243a69530cd66c93a","unresolved":false,"context_lines":[{"line_number":775,"context_line":"        required_traits \u003d {\u0027CUSTOM_TRAIT_A\u0027: traits_map[\u0027CUSTOM_TRAIT_A\u0027]}"},{"line_number":776,"context_line":"        self.assertEqual("},{"line_number":777,"context_line":"            [rps[1].id, rps[2].id],"},{"line_number":778,"context_line":"            rp_obj._get_provider_ids_having_any_trait(self.ctx,"},{"line_number":779,"context_line":"                                                      required_traits))"},{"line_number":780,"context_line":"        # require A, forbid B"},{"line_number":781,"context_line":"        forbiden_traits \u003d {\u0027CUSTOM_TRAIT_B\u0027: traits_map[\u0027CUSTOM_TRAIT_B\u0027]}"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_baa184aa","line":778,"updated":"2018-07-20 10:01:50.000000000","message":"Is this ordering going to be stable?","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":775,"context_line":"        required_traits \u003d {\u0027CUSTOM_TRAIT_A\u0027: traits_map[\u0027CUSTOM_TRAIT_A\u0027]}"},{"line_number":776,"context_line":"        self.assertEqual("},{"line_number":777,"context_line":"            [rps[1].id, rps[2].id],"},{"line_number":778,"context_line":"            rp_obj._get_provider_ids_having_any_trait(self.ctx,"},{"line_number":779,"context_line":"                                                      required_traits))"},{"line_number":780,"context_line":"        # require A, forbid B"},{"line_number":781,"context_line":"        forbiden_traits \u003d {\u0027CUSTOM_TRAIT_B\u0027: traits_map[\u0027CUSTOM_TRAIT_B\u0027]}"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_df278ac7","line":778,"in_reply_to":"5f7c97a3_baa184aa","updated":"2018-07-20 15:30:48.000000000","message":"Probably - it\u0027s likely to return in db order, which is (again probably) the order in which they were created, which is determined by the loop on L767 - but I agree this would be better if it didn\u0027t count on that. Use assertItemsEqual, or sort, or convert to sets.","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"34ad810e1704202bc1d0f871322533919528e684","unresolved":false,"context_lines":[{"line_number":775,"context_line":"        required_traits \u003d {\u0027CUSTOM_TRAIT_A\u0027: traits_map[\u0027CUSTOM_TRAIT_A\u0027]}"},{"line_number":776,"context_line":"        self.assertEqual("},{"line_number":777,"context_line":"            [rps[1].id, rps[2].id],"},{"line_number":778,"context_line":"            rp_obj._get_provider_ids_having_any_trait(self.ctx,"},{"line_number":779,"context_line":"                                                      required_traits))"},{"line_number":780,"context_line":"        # require A, forbid B"},{"line_number":781,"context_line":"        forbiden_traits \u003d {\u0027CUSTOM_TRAIT_B\u0027: traits_map[\u0027CUSTOM_TRAIT_B\u0027]}"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_82c4560f","line":778,"in_reply_to":"5f7c97a3_df278ac7","updated":"2018-07-20 16:26:28.000000000","message":"I think maybe it depends on the default select order of specific DB. : )\n\nand sqlite/mysql default order is order by id, and agree Eric, it\u0027s better to convert to sets.","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"66ae93866e2a4aba3d7e529713894038c164403e","unresolved":false,"context_lines":[{"line_number":790,"context_line":"            [rps[1].id, rps[2].id],"},{"line_number":791,"context_line":"            rp_obj._get_provider_ids_having_any_trait(self.ctx,"},{"line_number":792,"context_line":"                                                      required_traits))"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":""},{"line_number":795,"context_line":"class ResourceProviderListTestCase(tb.PlacementDbBaseTestCase):"},{"line_number":796,"context_line":"    def test_get_all_by_filters(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_5f7a9975","line":793,"updated":"2018-07-20 15:30:48.000000000","message":"Throw in a scenario with *just* forbidden traits (i.e. required_traits empty).","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"34ad810e1704202bc1d0f871322533919528e684","unresolved":false,"context_lines":[{"line_number":790,"context_line":"            [rps[1].id, rps[2].id],"},{"line_number":791,"context_line":"            rp_obj._get_provider_ids_having_any_trait(self.ctx,"},{"line_number":792,"context_line":"                                                      required_traits))"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":""},{"line_number":795,"context_line":"class ResourceProviderListTestCase(tb.PlacementDbBaseTestCase):"},{"line_number":796,"context_line":"    def test_get_all_by_filters(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_a2f07267","line":793,"in_reply_to":"5f7c97a3_5f7a9975","updated":"2018-07-20 16:26:28.000000000","message":"See my comments in \nhttps://review.openstack.org/#/c/584218/2/nova/api/openstack/placement/objects/resource_provider.py@3958","commit_id":"100d4ebe18e25af3957233d8f23ee39819ee14bc"}]}