)]}'
{"neutron/objects/qos/rule.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1cc7c258f9372603092259f38a48f40c4db706c0","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        # NOTE(ralonsoh): return True if:"},{"line_number":89,"context_line":"        #    - Is a port QoS policy (not a network QoS policy)"},{"line_number":90,"context_line":"        #    - Is not a network device (e.g. router) an is a network QoS policy"},{"line_number":91,"context_line":"        #      and there is no port QoS policy"},{"line_number":92,"context_line":"        return (is_port_policy or"},{"line_number":93,"context_line":"                (not is_network_device_port and is_network_policy_only))"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_0d8f2a24","line":91,"updated":"2017-01-26 07:25:02.000000000","message":"I think we can shorten this up a bit. There is only one case where we don\u0027t want to apply the policy: if there is no policy on the port and it is a network_device_port.\n\n    is_network_policy_only \u003d port[qos_consts.QOS_POLICY_ID] is None\n    is_network_device_port \u003d any(port[\u0027device_owner\u0027].startswith(prefix)\n                                 for prefix\n                                 in constants.DEVICE_OWNER_PREFIXES)\n    skip_apply \u003d is_network_policy_only and is_network_device_port\n    return not skip_apply","commit_id":"1b6fb2506d0626c0c1e46fa6b4009043fbff81a5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c7a3ea00bc61c92853a7929a3d06b86ee88816c6","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        # NOTE(ralonsoh): return True if:"},{"line_number":89,"context_line":"        #    - Is a port QoS policy (not a network QoS policy)"},{"line_number":90,"context_line":"        #    - Is not a network device (e.g. router) an is a network QoS policy"},{"line_number":91,"context_line":"        #      and there is no port QoS policy"},{"line_number":92,"context_line":"        return (is_port_policy or"},{"line_number":93,"context_line":"                (not is_network_device_port and is_network_policy_only))"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_860e9732","line":91,"in_reply_to":"3a461143_0d8f2a24","updated":"2017-01-26 09:00:24.000000000","message":"Reducing the logic we are missing some cases:\n1) test_should_apply_to_port_with_compute_port_and_net_policy:\n   If port QoS policy is not None BUT the QoS policy to apply is a network policy; port QoS policy takes precedence. With your logic this check is True, but should be False.\n\n2) test_should_apply_to_port_with_network_port_and_net_policy\n   The port QoS policy is None (that means the policy is from the network). But the port belongs to a router -\u003e policy should\u0027nt apply. With your logic this check is True, but should be False.","commit_id":"1b6fb2506d0626c0c1e46fa6b4009043fbff81a5"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"c4d5ccb7e042421f7165686a5ff1d98581e8c7d8","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        # NOTE(ralonsoh): return True if:"},{"line_number":89,"context_line":"        #    - Is a port QoS policy (not a network QoS policy)"},{"line_number":90,"context_line":"        #    - Is not a network device (e.g. router) an is a network QoS policy"},{"line_number":91,"context_line":"        #      and there is no port QoS policy"},{"line_number":92,"context_line":"        return (is_port_policy or"},{"line_number":93,"context_line":"                (not is_network_device_port and is_network_policy_only))"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_b746c178","line":91,"in_reply_to":"3a461143_860e9732","updated":"2017-02-15 13:31:33.000000000","message":"1. You\u0027re right, but see comments on latest patch.\n\n2. No, with my logic in this case \u0027is_network_policy_only\u0027 would be True and \u0027is_network_device_port\u0027 would be True so it would not apply.","commit_id":"1b6fb2506d0626c0c1e46fa6b4009043fbff81a5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a3f1dc0fe14bda039eead5e641a746975dcf6afd","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        # NOTE(ralonsoh): return True if:"},{"line_number":89,"context_line":"        #    - Is a port QoS policy (not a network QoS policy)"},{"line_number":90,"context_line":"        #    - Is not a network device (e.g. router) an is a network QoS policy"},{"line_number":91,"context_line":"        #      and there is no port QoS policy"},{"line_number":92,"context_line":"        return (is_port_policy or"},{"line_number":93,"context_line":"                (not is_network_device_port and is_network_policy_only))"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_b0ea4772","line":91,"in_reply_to":"fa31d9ce_b746c178","updated":"2017-02-15 14:39:11.000000000","message":"In 2) \"test_should_apply_to_port_with_network_port_and_net_policy\":\n  is_network_policy_only \u003d port[qos_consts.QOS_POLICY_ID] is None --\u003e False\n\nbecause port[qos_consts.QOS_POLICY_ID] \u003d POLICY_ID_A, according to the test.\n\nI executed the test using your code and this is the result: http://paste.openstack.org/show/599038/","commit_id":"1b6fb2506d0626c0c1e46fa6b4009043fbff81a5"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"468c91e109154e61e428353d95d12a50715cc38f","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        policy (network, or port). Eventually rules could override"},{"line_number":80,"context_line":"        this method, or we could make it abstract to allow different"},{"line_number":81,"context_line":"        rule behaviour."},{"line_number":82,"context_line":"        \"\"\""},{"line_number":83,"context_line":"        is_port_policy \u003d self.qos_policy_id \u003d\u003d port[qos_consts.QOS_POLICY_ID]"},{"line_number":84,"context_line":"        is_network_policy_only \u003d port[qos_consts.QOS_POLICY_ID] is None"},{"line_number":85,"context_line":"        is_network_device_port \u003d any(port[\u0027device_owner\u0027].startswith(prefix)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa31d9ce_b75c017e","line":82,"updated":"2017-02-15 13:31:52.000000000","message":"In your comments on PS1, you said that network policies are ignored if there is a port-level policy for compute ports as well. However, that\u0027s not how the previous logic works.\n\nLook at the old code on the left, the only time it would return False is if it was a network device.\n\nSo it seems like you are changing two things in this patch:\n\n1) Network ports can have a policy on the port, which will be applied.\n\n2) Network policies are skipped for compute ports as well if they have their own policy.\n\nNumber 2 is not what the bug is about. Why did you make that change?","commit_id":"26c75bf0335413c64afa06aa75648c8e4f0738f3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e042bfde2dcaff55258853f571bedd0b8b4a17d1","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        policy (network, or port). Eventually rules could override"},{"line_number":80,"context_line":"        this method, or we could make it abstract to allow different"},{"line_number":81,"context_line":"        rule behaviour."},{"line_number":82,"context_line":"        \"\"\""},{"line_number":83,"context_line":"        is_port_policy \u003d self.qos_policy_id \u003d\u003d port[qos_consts.QOS_POLICY_ID]"},{"line_number":84,"context_line":"        is_network_policy_only \u003d port[qos_consts.QOS_POLICY_ID] is None"},{"line_number":85,"context_line":"        is_network_device_port \u003d any(port[\u0027device_owner\u0027].startswith(prefix)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa31d9ce_6b8e202f","line":82,"in_reply_to":"fa31d9ce_b75c017e","updated":"2017-02-15 14:39:15.000000000","message":"\u003e In your comments on PS1, you said that network policies are ignored if there is a\n\u003e port-level policy for compute ports as well. However, that\u0027s not how the previous\n\u003e logic works.\nBecause the logic was wrong. From https://docs.openstack.org/mitaka/networking-guide/config-qos.html:\n\"You can attach networks to a QoS policy. The meaning of this is that any compute port connected to the network will use the network policy by default unless the port has a specific policy attached to it\"\n\n\n\u003e 1) Network ports can have a policy on the port, which will be applied.\nIt should but I\u0027ll ping ajo to check this point\n\n\n\u003e 2) Network policies are skipped for compute ports as well if they have their\n\u003e own policy. Number 2 is not what the bug is about. Why did you make that change?\nThe same reason: the previous logic was wrong. But, of course, I\u0027m going to ping ajo to be 100% sure.","commit_id":"26c75bf0335413c64afa06aa75648c8e4f0738f3"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"62760aa5d7380c250b4a80ce1332a175fb58bca2","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                                     in constants.DEVICE_OWNER_PREFIXES)"},{"line_number":88,"context_line":"        # NOTE(ralonsoh): return True if:"},{"line_number":89,"context_line":"        #    - Is a port QoS policy (not a network QoS policy)"},{"line_number":90,"context_line":"        #    - Is not a network device (e.g. router) an is a network QoS policy"},{"line_number":91,"context_line":"        #      and there is no port QoS policy"},{"line_number":92,"context_line":"        return (is_port_policy or"},{"line_number":93,"context_line":"                (not is_network_device_port and is_network_policy_only))"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_4094a743","line":90,"range":{"start_line":90,"start_character":53,"end_line":90,"end_character":55},"updated":"2017-02-16 13:40:22.000000000","message":"and","commit_id":"f755a2b857b4ee2d05046b2213f95bb875e16e7d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"53d76b1316d6fe4bc66d06b0dd8a3a9c6d3b1799","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                                     in constants.DEVICE_OWNER_PREFIXES)"},{"line_number":88,"context_line":"        # NOTE(ralonsoh): return True if:"},{"line_number":89,"context_line":"        #    - Is a port QoS policy (not a network QoS policy)"},{"line_number":90,"context_line":"        #    - Is not a network device (e.g. router) an is a network QoS policy"},{"line_number":91,"context_line":"        #      and there is no port QoS policy"},{"line_number":92,"context_line":"        return (is_port_policy or"},{"line_number":93,"context_line":"                (not is_network_device_port and is_network_policy_only))"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_58801447","line":90,"range":{"start_line":90,"start_character":53,"end_line":90,"end_character":55},"in_reply_to":"fa31d9ce_4094a743","updated":"2017-02-16 18:32:32.000000000","message":"Done","commit_id":"f755a2b857b4ee2d05046b2213f95bb875e16e7d"}],"neutron/tests/unit/objects/qos/test_rule.py":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"62760aa5d7380c250b4a80ce1332a175fb58bca2","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_should_apply_to_port_with_compute_port_and_net_policy(self):"},{"line_number":60,"context_line":"        # NOTE(ralonsoh): in this case the port has a port QoS policy; the"},{"line_number":61,"context_line":"        # network QoS policy can\u0027t be applied."},{"line_number":62,"context_line":"        self._test_should_apply_to_port("},{"line_number":63,"context_line":"            rule_policy_id\u003dPOLICY_ID_B,"},{"line_number":64,"context_line":"            port_policy_id\u003dPOLICY_ID_A,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_5b501c5f","line":61,"updated":"2017-02-16 13:40:22.000000000","message":"i thought \"prefer port policy\" logic was in handle_port. [1]\ncan you explain in which case this change matters?\n\n[1] https://github.com/openstack/neutron/blob/0480a07b49275d98181358d11836a8112dce6ee5/neutron/agent/l2/extensions/qos.py#L233","commit_id":"f755a2b857b4ee2d05046b2213f95bb875e16e7d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"53d76b1316d6fe4bc66d06b0dd8a3a9c6d3b1799","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_should_apply_to_port_with_compute_port_and_net_policy(self):"},{"line_number":60,"context_line":"        # NOTE(ralonsoh): in this case the port has a port QoS policy; the"},{"line_number":61,"context_line":"        # network QoS policy can\u0027t be applied."},{"line_number":62,"context_line":"        self._test_should_apply_to_port("},{"line_number":63,"context_line":"            rule_policy_id\u003dPOLICY_ID_B,"},{"line_number":64,"context_line":"            port_policy_id\u003dPOLICY_ID_A,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa31d9ce_33b3810c","line":61,"in_reply_to":"fa31d9ce_5b501c5f","updated":"2017-02-16 18:32:32.000000000","message":"As you effectivly pointed out, this case should never happend because of the logic in QosAgentExtension.handle_port.\n\nBut this test checks the logic of this function only and this is one of the possible logic scenarios. In the case, if a new policy is checked and is not the port qos policy, that means this is a network qos policy and can\u0027t be applied. Of course, as I said, this case should never happend because of the logic in handle_port. But it doesn\u0027t mean we should keep the logic of this function incorrect.","commit_id":"f755a2b857b4ee2d05046b2213f95bb875e16e7d"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5ee6b8494cff2b16b8002a0ca207007217f2206d","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            device_owner\u003dconstants.DEVICE_OWNER_ROUTER_INTF,"},{"line_number":43,"context_line":"            expected_result\u003dFalse)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_should_apply_to_port_with_network_port_and_only_net_policy(self):"},{"line_number":46,"context_line":"        self._test_should_apply_to_port("},{"line_number":47,"context_line":"            rule_policy_id\u003dPOLICY_ID_B,"},{"line_number":48,"context_line":"            port_policy_id\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_e42afd2e","line":45,"updated":"2017-02-16 20:03:07.000000000","message":"shouldn\u0027t it be \"test_should_NOT_apply_to_port...\" ?","commit_id":"acba3d3610cfcfc2c212f1beae16baa31bc495fd"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"435f1c55f1b99cfcaba7961903b50e16efff7e78","unresolved":false,"context_lines":[{"line_number":42,"context_line":"            device_owner\u003dconstants.DEVICE_OWNER_ROUTER_INTF,"},{"line_number":43,"context_line":"            expected_result\u003dFalse)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_should_apply_to_port_with_network_port_and_only_net_policy(self):"},{"line_number":46,"context_line":"        self._test_should_apply_to_port("},{"line_number":47,"context_line":"            rule_policy_id\u003dPOLICY_ID_B,"},{"line_number":48,"context_line":"            port_policy_id\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"da36d5c6_c69433ea","line":45,"in_reply_to":"fa31d9ce_e42afd2e","updated":"2017-02-18 15:33:26.000000000","message":"The name of the function to be tested is \"should_apply_to_port\"; that\u0027s why I\u0027m using this name.","commit_id":"acba3d3610cfcfc2c212f1beae16baa31bc495fd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5ee6b8494cff2b16b8002a0ca207007217f2206d","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            device_owner\u003dconstants.DEVICE_OWNER_ROUTER_INTF,"},{"line_number":57,"context_line":"            expected_result\u003dTrue)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_should_apply_to_port_with_compute_port_and_net_policy(self):"},{"line_number":60,"context_line":"        # NOTE(ralonsoh): in this case the port has a port QoS policy; the"},{"line_number":61,"context_line":"        # network QoS policy can\u0027t be applied."},{"line_number":62,"context_line":"        self._test_should_apply_to_port("}],"source_content_type":"text/x-python","patch_set":7,"id":"fa31d9ce_04d71124","line":59,"updated":"2017-02-16 20:03:07.000000000","message":"same here: shouldn\u0027t it be \"test_should_not_apply...\"?","commit_id":"acba3d3610cfcfc2c212f1beae16baa31bc495fd"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"435f1c55f1b99cfcaba7961903b50e16efff7e78","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            device_owner\u003dconstants.DEVICE_OWNER_ROUTER_INTF,"},{"line_number":57,"context_line":"            expected_result\u003dTrue)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_should_apply_to_port_with_compute_port_and_net_policy(self):"},{"line_number":60,"context_line":"        # NOTE(ralonsoh): in this case the port has a port QoS policy; the"},{"line_number":61,"context_line":"        # network QoS policy can\u0027t be applied."},{"line_number":62,"context_line":"        self._test_should_apply_to_port("}],"source_content_type":"text/x-python","patch_set":7,"id":"da36d5c6_e6993721","line":59,"in_reply_to":"fa31d9ce_04d71124","updated":"2017-02-18 15:33:26.000000000","message":"ditto","commit_id":"acba3d3610cfcfc2c212f1beae16baa31bc495fd"}]}