)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b69373ec8eac6f59abce8aae652eebf5e4686526","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Tetsuro Nakamura \u003cnakamura.tetsuro@lab.ntt.co.jp\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-05-28 23:42:46 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"placement: Use INNER JOIN for requied traits"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Getting allocation candidates, placement calls _get_trees_matching_all"},{"line_number":10,"context_line":"to get all trees that satisfy user requests. The function consisted of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_59c0fae0","line":7,"range":{"start_line":7,"start_character":30,"end_line":7,"end_character":37},"updated":"2018-06-08 21:22:28.000000000","message":"required","commit_id":"6ca1658a69762f34e830b9ced7262fd870e49906"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"9a4fe62ea8599ad6143c3ee4f1dce0b3be0221f7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Tetsuro Nakamura \u003cnakamura.tetsuro@lab.ntt.co.jp\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-05-29 01:16:30 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"placement: Use INNER JOIN for requied traits"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Getting allocation candidates, placement calls _get_trees_matching_all"},{"line_number":10,"context_line":"to get all trees that satisfy user requests. The function consisted of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_3a4a8b26","line":7,"range":{"start_line":7,"start_character":30,"end_line":7,"end_character":37},"updated":"2018-06-11 21:56:18.000000000","message":"nit: required\n\n(I appreciate the irony)","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"68eef3a342fb7b26147c027aaee45ed372cc03bf","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Tetsuro Nakamura \u003cnakamura.tetsuro@lab.ntt.co.jp\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-05-29 01:16:30 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"placement: Use INNER JOIN for requied traits"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Getting allocation candidates, placement calls _get_trees_matching_all"},{"line_number":10,"context_line":"to get all trees that satisfy user requests. The function consisted of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_31cf0e80","line":7,"range":{"start_line":7,"start_character":30,"end_line":7,"end_character":37},"in_reply_to":"5f7c97a3_3a4a8b26","updated":"2018-06-12 02:50:34.000000000","message":"Oops, will fix if I need to respin...","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"}],"nova/api/openstack/placement/objects/resource_provider.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f96ca68c3555e8c83005b7116a0f70e16855f43","unresolved":false,"context_lines":[{"line_number":2890,"context_line":"    #   ON rp.id \u003d rptt.resource_provider_id"},{"line_number":2891,"context_line":"    #   AND rptt.trait_id IN ($REQUIRED_TRAIT_IDS)"},{"line_number":2892,"context_line":"    #   # Only if we have forbidden_traits..."},{"line_number":2893,"context_line":"    #   LEFT JOIN resource_provider_traits AS rptt_forbid"},{"line_number":2894,"context_line":"    #   ON rp.id \u003d rptt_forbid.resource_provider_id"},{"line_number":2895,"context_line":"    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS)"},{"line_number":2896,"context_line":"    #   WHERE rp.id IN ($RP_IDS)"},{"line_number":2897,"context_line":"    #   # Only if we have forbidden traits..."},{"line_number":2898,"context_line":"    #   AND rptt_forbid.resource_provider_id IS NULL"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_0f70bd71","line":2895,"range":{"start_line":2893,"start_character":8,"end_line":2895,"end_character":58},"updated":"2018-06-11 12:05:59.000000000","message":"Don\u0027t we need a negation somewhere? This reads for me like \u0027give me providers that has forbidden traits\u0027.","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f96ca68c3555e8c83005b7116a0f70e16855f43","unresolved":false,"context_lines":[{"line_number":2894,"context_line":"    #   ON rp.id \u003d rptt_forbid.resource_provider_id"},{"line_number":2895,"context_line":"    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS)"},{"line_number":2896,"context_line":"    #   WHERE rp.id IN ($RP_IDS)"},{"line_number":2897,"context_line":"    #   # Only if we have forbidden traits..."},{"line_number":2898,"context_line":"    #   AND rptt_forbid.resource_provider_id IS NULL"},{"line_number":2899,"context_line":"    #   GROUP BY rp.root_provider_id"},{"line_number":2900,"context_line":"    #   # Only if have required traits..."},{"line_number":2901,"context_line":"    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8f092dde","line":2898,"range":{"start_line":2897,"start_character":1,"end_line":2898,"end_character":52},"updated":"2018-06-11 12:05:59.000000000","message":"The comment and the commented line contradicts according to my parser. \u0027Only if we have forbidden traits\u0027 and then we ask for  resource_provider_id that is NULL","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"68eef3a342fb7b26147c027aaee45ed372cc03bf","unresolved":false,"context_lines":[{"line_number":2894,"context_line":"    #   ON rp.id \u003d rptt_forbid.resource_provider_id"},{"line_number":2895,"context_line":"    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS)"},{"line_number":2896,"context_line":"    #   WHERE rp.id IN ($RP_IDS)"},{"line_number":2897,"context_line":"    #   # Only if we have forbidden traits..."},{"line_number":2898,"context_line":"    #   AND rptt_forbid.resource_provider_id IS NULL"},{"line_number":2899,"context_line":"    #   GROUP BY rp.root_provider_id"},{"line_number":2900,"context_line":"    #   # Only if have required traits..."},{"line_number":2901,"context_line":"    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_d19fbab1","line":2898,"range":{"start_line":2897,"start_character":1,"end_line":2898,"end_character":52},"in_reply_to":"5f7c97a3_37f866c2","updated":"2018-06-12 02:50:34.000000000","message":"Yup, let’s say we have two tables like;\n\n    -----\n    rp.id\n    -----\n      1\n      2\n      3\n    -----\n\n    -------------------------------------------------------\n    rptt_forbid.resource_provider_id   rptt_forbid.trait_id \n    -------------------------------------------------------\n                     1                         10\n                     2                         11\n    -------------------------------------------------------\n \nAnd let me say we don’t want the trait whose id is 11.\n\nIf inner join,\n    #   resource provider AS rp\n    #   JOIN resource_provider_traits AS rptt_forbid\n    #   ON rp.id \u003d rptt_forbid.resource_provider_id\n    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS\u003d11)\n\nwe get a row which satisfies the conditions.\n\n    ----------------------------\n    rp.id   rptt_forbid.trait_id \n    ----------------------------\n       2                 11\n    ----------------------------\n\nBut if outer join, it calls all the rows on “rp” table (rp.id\u003d1 and 3) setting the right table columns to NULL:\n\n    ----------------------------\n    rp.id   rptt_forbid.trait_id \n    ----------------------------\n       2           11\n       1          NULL\n       3          NULL\n    ----------------------------\n\nWatch out that rptt_forbid.trait_id has NULL value on rp.id\u003d3 while it has a trait whose id 10. That\u0027s how left join works. Thus we can get rps without trait id 11 by specifying NULL explicitly.\n\nCool. Thanks a lot to Jay for help.","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"afccd66e3e5c1e056c6fbbb412f318b143a0ef0c","unresolved":false,"context_lines":[{"line_number":2894,"context_line":"    #   ON rp.id \u003d rptt_forbid.resource_provider_id"},{"line_number":2895,"context_line":"    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS)"},{"line_number":2896,"context_line":"    #   WHERE rp.id IN ($RP_IDS)"},{"line_number":2897,"context_line":"    #   # Only if we have forbidden traits..."},{"line_number":2898,"context_line":"    #   AND rptt_forbid.resource_provider_id IS NULL"},{"line_number":2899,"context_line":"    #   GROUP BY rp.root_provider_id"},{"line_number":2900,"context_line":"    #   # Only if have required traits..."},{"line_number":2901,"context_line":"    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_37f866c2","line":2898,"range":{"start_line":2897,"start_character":1,"end_line":2898,"end_character":52},"in_reply_to":"5f7c97a3_8166d950","updated":"2018-06-11 20:48:35.000000000","message":"That\u0027s my understanding too","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0b9e296825a8211514f43046e7fed02602bdf388","unresolved":false,"context_lines":[{"line_number":2894,"context_line":"    #   ON rp.id \u003d rptt_forbid.resource_provider_id"},{"line_number":2895,"context_line":"    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS)"},{"line_number":2896,"context_line":"    #   WHERE rp.id IN ($RP_IDS)"},{"line_number":2897,"context_line":"    #   # Only if we have forbidden traits..."},{"line_number":2898,"context_line":"    #   AND rptt_forbid.resource_provider_id IS NULL"},{"line_number":2899,"context_line":"    #   GROUP BY rp.root_provider_id"},{"line_number":2900,"context_line":"    #   # Only if have required traits..."},{"line_number":2901,"context_line":"    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8166d950","line":2898,"range":{"start_line":2897,"start_character":1,"end_line":2898,"end_character":52},"in_reply_to":"5f7c97a3_8f092dde","updated":"2018-06-11 18:26:39.000000000","message":"Okay, my understanding is that the purpose of the LEFT JOIN is to create a column where the forbidden trait is requested, so that that column will be NULL for a given row when the trait is *absent*, which is why we select for that condition.\n\nIOW, we want rows where we *didn\u0027t* find the forbidden traits.","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b3473a446b121cfae6ea06be4b03461062a8198","unresolved":false,"context_lines":[{"line_number":2894,"context_line":"    #   ON rp.id \u003d rptt_forbid.resource_provider_id"},{"line_number":2895,"context_line":"    #   AND rptt_forbid.trait_id IN ($FORBIDDEN_TRAIT_IDS)"},{"line_number":2896,"context_line":"    #   WHERE rp.id IN ($RP_IDS)"},{"line_number":2897,"context_line":"    #   # Only if we have forbidden traits..."},{"line_number":2898,"context_line":"    #   AND rptt_forbid.resource_provider_id IS NULL"},{"line_number":2899,"context_line":"    #   GROUP BY rp.root_provider_id"},{"line_number":2900,"context_line":"    #   # Only if have required traits..."},{"line_number":2901,"context_line":"    #   HAVING COUNT(DISTINCT rptt.trait_id) \u003d\u003d $NUM_REQUIRED_TRAITS"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_f69e5e04","line":2898,"range":{"start_line":2897,"start_character":1,"end_line":2898,"end_character":52},"in_reply_to":"5f7c97a3_d19fbab1","updated":"2018-06-12 14:17:43.000000000","message":"Thank you all for the explanation. It is a lot clearer now.","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"}],"nova/tests/functional/api/openstack/placement/db/test_allocation_candidates.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2f96ca68c3555e8c83005b7116a0f70e16855f43","unresolved":false,"context_lines":[{"line_number":2327,"context_line":""},{"line_number":2328,"context_line":"    def test_get_trees_with_traits(self):"},{"line_number":2329,"context_line":"        \"\"\"Creates a few provider trees having different traits and tests the"},{"line_number":2330,"context_line":"        _get_trees_with_traits() utility function to ensure that only the"},{"line_number":2331,"context_line":"        root provider IDs of matching traits are returned."},{"line_number":2332,"context_line":"        \"\"\""},{"line_number":2333,"context_line":"        # We are setting up 6 trees of providers with following traits:"},{"line_number":2334,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5222dea4","line":2331,"range":{"start_line":2330,"start_character":53,"end_line":2331,"end_character":57},"updated":"2018-06-11 12:05:59.000000000","message":"Is there any reason not to check the returned rp_id as well? It seems to me that the code in [1] check for both the root and the matching rp_id.\n\n[1]https://review.openstack.org/#/c/573113/3/nova/api/openstack/placement/objects/resource_provider.py@3079","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"68eef3a342fb7b26147c027aaee45ed372cc03bf","unresolved":false,"context_lines":[{"line_number":2327,"context_line":""},{"line_number":2328,"context_line":"    def test_get_trees_with_traits(self):"},{"line_number":2329,"context_line":"        \"\"\"Creates a few provider trees having different traits and tests the"},{"line_number":2330,"context_line":"        _get_trees_with_traits() utility function to ensure that only the"},{"line_number":2331,"context_line":"        root provider IDs of matching traits are returned."},{"line_number":2332,"context_line":"        \"\"\""},{"line_number":2333,"context_line":"        # We are setting up 6 trees of providers with following traits:"},{"line_number":2334,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_91ddc242","line":2331,"range":{"start_line":2330,"start_character":53,"end_line":2331,"end_character":57},"in_reply_to":"5f7c97a3_5222dea4","updated":"2018-06-12 02:50:34.000000000","message":"With the SQL query created so far, we can detect which trees have a combination of rps which satisfies the trait requirements, but it can\u0027t return the exact combinations of rps, so all we need to check here is only root providers.\n\nYou\u0027re right that the code you linked now checks for both the root and the matching rp_id, but it can be refactored to only check root id.","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b3473a446b121cfae6ea06be4b03461062a8198","unresolved":false,"context_lines":[{"line_number":2327,"context_line":""},{"line_number":2328,"context_line":"    def test_get_trees_with_traits(self):"},{"line_number":2329,"context_line":"        \"\"\"Creates a few provider trees having different traits and tests the"},{"line_number":2330,"context_line":"        _get_trees_with_traits() utility function to ensure that only the"},{"line_number":2331,"context_line":"        root provider IDs of matching traits are returned."},{"line_number":2332,"context_line":"        \"\"\""},{"line_number":2333,"context_line":"        # We are setting up 6 trees of providers with following traits:"},{"line_number":2334,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_16643a21","line":2331,"range":{"start_line":2330,"start_character":53,"end_line":2331,"end_character":57},"in_reply_to":"5f7c97a3_91ddc242","updated":"2018-06-12 14:17:43.000000000","message":"Fair enough. Thanks for the explanation.","commit_id":"5447a695e4406a2dd94166003643cc11c3377e38"}]}