)]}'
{"neutron/common/exceptions.py":[{"author":{"_account_id":18051,"name":"David Shaughnessy","email":"david.shaughnessy@intel.com","username":"davidsha"},"change_message_id":"6b9f5379f210455da288e44328e259fc94e501ac","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"class QoSRuleParameterConflict(e.Conflict):"},{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as minimum \""},{"line_number":162,"context_line":"                \"bandwidth %(min_kbps)s defined for the policy is less \""},{"line_number":163,"context_line":"                \"than requested maximum bandwidth %(max_kbps)s\")"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_82ca1d4e","line":162,"range":{"start_line":162,"start_character":66,"end_line":162,"end_character":70},"updated":"2017-03-07 10:59:01.000000000","message":"s/less/more","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":18051,"name":"David Shaughnessy","email":"david.shaughnessy@intel.com","username":"davidsha"},"change_message_id":"6b9f5379f210455da288e44328e259fc94e501ac","unresolved":false,"context_lines":[{"line_number":160,"context_line":"class QoSRuleParameterConflict(e.Conflict):"},{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as minimum \""},{"line_number":162,"context_line":"                \"bandwidth %(min_kbps)s defined for the policy is less \""},{"line_number":163,"context_line":"                \"than requested maximum bandwidth %(max_kbps)s\")"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_42e745e4","line":163,"range":{"start_line":163,"start_character":17,"end_line":163,"end_character":22},"updated":"2017-03-07 10:59:01.000000000","message":"s/than/than the","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9514ad5c44d54b64b061b34d3a0656c213f69e77","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as the \""},{"line_number":162,"context_line":"                \"policy has an existing rule which defines the \""},{"line_number":163,"context_line":"                \"bandwidth limit of %(min_kbps)s and the rule requests \""},{"line_number":164,"context_line":"                \"for a bandwith limit of %(max_kbps)s\")"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_ec72ba48","line":164,"updated":"2017-03-08 08:01:00.000000000","message":"Your commit message is only for case when user wants to add bandwidth limit rule if minimum bandwidth limit already exists and is higher.\nYou should do it more \"generic\" or maybe do two different exceptions.","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"5f9053952929ea5c6e3c3fe81a515e410cc1588d","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as the \""},{"line_number":162,"context_line":"                \"policy has an existing rule which defines the \""},{"line_number":163,"context_line":"                \"bandwidth limit of %(min_kbps)s and the rule requests \""},{"line_number":164,"context_line":"                \"for a bandwith limit of %(max_kbps)s\")"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_46061ecf","line":164,"in_reply_to":"9a30ddce_ec72ba48","updated":"2017-03-09 04:19:18.000000000","message":"Done","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"16e895d1593e7250a2aab102732c1d69bbf1779a","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as the \""},{"line_number":162,"context_line":"                \"policy has an existing %(rule_type)s rule which \""},{"line_number":163,"context_line":"                \"defines the minimum bandwidth limit of %(min_kbps)s \""},{"line_number":164,"context_line":"                \"and the rule requests for a bandwith of %(max_kbps)s.\")"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_92b84ce7","line":164,"updated":"2017-03-09 07:42:24.000000000","message":"Sorry but this error message still is valid only for case when user tries to add bandwidth_limit rule with limit lower than existing minimum_bandwidth.\nIn opposite case it will be wrong.","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"ad377a446a182749337588dea4ea01167ba82050","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as the \""},{"line_number":162,"context_line":"                \"policy has an existing %(rule_type)s rule which \""},{"line_number":163,"context_line":"                \"defines the minimum bandwidth limit of %(min_kbps)s \""},{"line_number":164,"context_line":"                \"and the rule requests for a bandwith of %(max_kbps)s.\")"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_e40a4125","line":164,"in_reply_to":"9a30ddce_6431f159","updated":"2017-03-13 00:57:09.000000000","message":"Miguel : Yeah, thats pretty informative, but the conflict may not be describing WHY the conflict actually occurred ...\n\nThat was why I wanted to mention the limit keyword, or any other word which states that the action is restricted.\n\nHow about:\n\n  Unable to add the %(rule_type)s rule with value %(rule_value)s to the policy %(policy_id)s as the existing rule of type %(old_rule)s restricts the bandwidth to %(existing_value)s","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"5e592fb6f17059cdde3f0d3e2e4ef0a6aceafb08","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as the \""},{"line_number":162,"context_line":"                \"policy has an existing %(rule_type)s rule which \""},{"line_number":163,"context_line":"                \"defines the minimum bandwidth limit of %(min_kbps)s \""},{"line_number":164,"context_line":"                \"and the rule requests for a bandwith of %(max_kbps)s.\")"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_c961d606","line":164,"in_reply_to":"9a30ddce_92b84ce7","updated":"2017-03-12 20:02:02.000000000","message":"Slaweq : I think the Rule_Type here would inform the user what type of rule is blocking this change.\n\nIf you have any other rewording suggestions, it would be greatly appreciated.","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"9f32212e02fc0a1fa37420992c7e194b54706431","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    message \u003d _(\"Unable to add rule to policy %(policy_id)s as the \""},{"line_number":162,"context_line":"                \"policy has an existing %(rule_type)s rule which \""},{"line_number":163,"context_line":"                \"defines the minimum bandwidth limit of %(min_kbps)s \""},{"line_number":164,"context_line":"                \"and the rule requests for a bandwith of %(max_kbps)s.\")"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_6431f159","line":164,"in_reply_to":"9a30ddce_c961d606","updated":"2017-03-12 23:40:40.000000000","message":"How about:\n\n\"Unable to add to policy id %(policy_id)s new rule of type %(new_rule_type)s with value %(new_rule_value)s because it conflicts with existing rule of type %(existing_rule_type)s with value %(existing_rule_value)s.\"","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"19b9203cb80a8dcf7b37dbcbeff64232ec683c2f","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    message \u003d _(\"Unable to add the rule with value %(rule_value)s to the \""},{"line_number":162,"context_line":"                \"policy %(policy_id)s as the existing rule of type \""},{"line_number":163,"context_line":"                \"%(existing_rule)s restricts the bandwidth to \""},{"line_number":164,"context_line":"                \"%(existing_value)s.\")"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"class InvalidExtensionEnv(e.BadRequest):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_4fb3210a","line":164,"updated":"2017-03-13 07:41:23.000000000","message":"For me now is much better. Thx for change :)","commit_id":"8c4789b862a9e3318b414d6140eefb6945b7695e"}],"neutron/objects/qos/object_checker.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9514ad5c44d54b64b061b34d3a0656c213f69e77","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import neutron.services.qos.qos_consts"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class ObjectChecker:"},{"line_number":22,"context_line":"    \"\"\"Implementation of the Neutron QoS Object checker."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    This class houses the various functions which resolves the semantics"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_ac9fe24e","line":21,"updated":"2017-03-08 08:01:00.000000000","message":"I\u0027m not sure if this check requires separate class for it.\nIMHO it could be done e.g. in https://github.com/openstack/neutron/blob/master/neutron/services/qos/drivers/manager.py for example\n\nOr if You really need separate file for it, then maybe simple function would be enough? Here I don\u0027t see any reason why it should be enapsulated in separate class","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"2a6ed8be0b998417e4d3a0013daeae3bfc95f614","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import neutron.services.qos.qos_consts"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class ObjectChecker:"},{"line_number":22,"context_line":"    \"\"\"Implementation of the Neutron QoS Object checker."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    This class houses the various functions which resolves the semantics"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_d041000d","line":21,"in_reply_to":"9a30ddce_ac9fe24e","updated":"2017-03-08 12:11:07.000000000","message":"Sorry, that was my idea. My thought is that eventually this is going to grow and grow with more rules. But as you say, a class is not necessary, may be just a method of the object_checker module would be more than enough.","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"5f9053952929ea5c6e3c3fe81a515e410cc1588d","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import neutron.services.qos.qos_consts"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class ObjectChecker:"},{"line_number":22,"context_line":"    \"\"\"Implementation of the Neutron QoS Object checker."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    This class houses the various functions which resolves the semantics"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_86101603","line":21,"in_reply_to":"9a30ddce_d041000d","updated":"2017-03-09 04:19:18.000000000","message":"Done","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"16e895d1593e7250a2aab102732c1d69bbf1779a","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.services.qos import qos_consts"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"def check_rule_conflict(policy, rule_data):"},{"line_number":22,"context_line":"    \"\"\"Implementation of the Neutron QoS Object checker."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    This function resolves the conflicts among various QoS policies and rules."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_d2f564cb","line":21,"updated":"2017-03-09 07:42:24.000000000","message":"IMHO name of this function should be more \"verbose\" and tell that it\u0027s checking only conflicts in bandwidth values in rules","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"ad377a446a182749337588dea4ea01167ba82050","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.services.qos import qos_consts"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"def check_rule_conflict(policy, rule_data):"},{"line_number":22,"context_line":"    \"\"\"Implementation of the Neutron QoS Object checker."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    This function resolves the conflicts among various QoS policies and rules."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_0406751f","line":21,"in_reply_to":"9a30ddce_d2f564cb","updated":"2017-03-13 00:57:09.000000000","message":"Ok, that would be helpful for others as well","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"16e895d1593e7250a2aab102732c1d69bbf1779a","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    This function resolves the conflicts among various QoS policies and rules."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    \"\"\""},{"line_number":27,"context_line":"    # if no rule is defined for the policy, the for loop wont execute"},{"line_number":28,"context_line":"        for rule in policy.rules:"},{"line_number":29,"context_line":"            if rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH:"},{"line_number":30,"context_line":"                if \"max_kbps\" in rule_data and ("}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_5230949e","line":27,"updated":"2017-03-09 07:42:24.000000000","message":"You should first check type of rule passed in rule_data - if it\u0027s e.g. dscp marking rule then other checks are not necessary here","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"ad377a446a182749337588dea4ea01167ba82050","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    This function resolves the conflicts among various QoS policies and rules."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    \"\"\""},{"line_number":27,"context_line":"    # if no rule is defined for the policy, the for loop wont execute"},{"line_number":28,"context_line":"        for rule in policy.rules:"},{"line_number":29,"context_line":"            if rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH:"},{"line_number":30,"context_line":"                if \"max_kbps\" in rule_data and ("}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_2401b906","line":27,"in_reply_to":"9a30ddce_5230949e","updated":"2017-03-13 00:57:09.000000000","message":"Agreed, will add it in the next iteration","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"9f32212e02fc0a1fa37420992c7e194b54706431","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    \"\"\""},{"line_number":27,"context_line":"    # if no rule is defined for the policy, the for loop wont execute"},{"line_number":28,"context_line":"        for rule in policy.rules:"},{"line_number":29,"context_line":"            if rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH:"},{"line_number":30,"context_line":"                if \"max_kbps\" in rule_data and ("},{"line_number":31,"context_line":"                   int(rule.min_kbps) \u003e int(rule_data[\"max_kbps\"])):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_c470dd0d","line":28,"updated":"2017-03-12 23:40:40.000000000","message":"Please don\u0027t forget this is overindented. That is why you go a pep8 failure ;-)","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"ad377a446a182749337588dea4ea01167ba82050","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    \"\"\""},{"line_number":27,"context_line":"    # if no rule is defined for the policy, the for loop wont execute"},{"line_number":28,"context_line":"        for rule in policy.rules:"},{"line_number":29,"context_line":"            if rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH:"},{"line_number":30,"context_line":"                if \"max_kbps\" in rule_data and ("},{"line_number":31,"context_line":"                   int(rule.min_kbps) \u003e int(rule_data[\"max_kbps\"])):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_3f02a00a","line":28,"in_reply_to":"9a30ddce_c470dd0d","updated":"2017-03-13 00:57:09.000000000","message":"eeks....","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"9f32212e02fc0a1fa37420992c7e194b54706431","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                   int(rule.min_kbps) \u003e int(rule_data[\"max_kbps\"])):"},{"line_number":32,"context_line":"                        raise n_exc.QoSRuleParameterConflict("},{"line_number":33,"context_line":"                            policy_id\u003dpolicy[\u0027id\u0027],"},{"line_number":34,"context_line":"                            rule_type\u003drule.rule_type,"},{"line_number":35,"context_line":"                            min_kbps\u003drule.min_kbps,"},{"line_number":36,"context_line":"                            max_kbps\u003drule_data[\"max_kbps\"])"},{"line_number":37,"context_line":"            elif rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_BANDWIDTH_LIMIT:"},{"line_number":38,"context_line":"                if \"min_kbps\" in rule_data and ("},{"line_number":39,"context_line":"                   int(rule.max_kbps) \u003c int(rule_data[\"min_kbps\"])):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_840b457c","line":36,"range":{"start_line":34,"start_character":0,"end_line":36,"end_character":59},"updated":"2017-03-12 23:40:40.000000000","message":"If you follow suggestion to https://review.openstack.org/#/c/442375/4/neutron/common/exceptions.py@164, then you would change this to:\n\n    new_rule_type\u003dqos_consts.RULE_TYPE_BANDWIDTH_LIMIT,\n    new_rule_value\u003drule_data[\"max_kbps\"],\n    existing_rule_type\u003drule.rule_type,\n    existing_rule_value\u003drule.min_kbps)","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"9f32212e02fc0a1fa37420992c7e194b54706431","unresolved":false,"context_lines":[{"line_number":39,"context_line":"                   int(rule.max_kbps) \u003c int(rule_data[\"min_kbps\"])):"},{"line_number":40,"context_line":"                        raise n_exc.QoSRuleParameterConflict("},{"line_number":41,"context_line":"                            policy_id\u003dpolicy[\u0027id\u0027],"},{"line_number":42,"context_line":"                            rule_type\u003drule.rule_type,"},{"line_number":43,"context_line":"                            min_kbps\u003drule.max_kbps,"},{"line_number":44,"context_line":"                            max_kbps\u003drule_data[\"min_kbps\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_647f31e0","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":59},"updated":"2017-03-12 23:40:40.000000000","message":"If you follow suggestion to https://review.openstack.org/#/c/442375/4/neutron/common/exceptions.py@164, then you would change this to:\n\n    new_rule_type\u003dqos_consts.RULE_TYPE_MINIMUM_BANDWIDTH,\n    new_rule_value\u003drule_data[\"min_kbps\"],\n    existing_rule_type\u003drule.rule_type,\n    existing_rule_value\u003drule.max_kbps)","commit_id":"fc284288fb76396621d4fe3dc1dfc5489ca207dd"},{"author":{"_account_id":18051,"name":"David Shaughnessy","email":"david.shaughnessy@intel.com","username":"davidsha"},"change_message_id":"f7f8b8eec07c9abe5352700617d679e09cc14431","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2017-18 NEC Technologies India Pvt Ltd."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a1ced50_7de1579a","line":1,"updated":"2017-03-23 17:18:01.000000000","message":"Would a more appropriate name for this file be qos_policy_validator or such?\n\nobject_checker just seems too vague a name.","commit_id":"a665aded93391770c7685fa289e1cbcc08e896b7"}],"neutron/objects/qos/qos_policy_validator.py":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"20a3132a234c48bc631bde10f86aa27fa3a5402c","unresolved":false,"context_lines":[{"line_number":21,"context_line":"def check_bandwidth_rule_conflict(policy, rule_data):"},{"line_number":22,"context_line":"    \"\"\"Implementation of the Neutron QoS Object checker."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    This function resolves the conflicts among various QoS policies and rules."},{"line_number":25,"context_line":"    \"\"\""},{"line_number":26,"context_line":"    for rule in policy.rules:"},{"line_number":27,"context_line":"        if rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_DSCP_MARKING:"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_a633c713","line":24,"range":{"start_line":24,"start_character":18,"end_line":24,"end_character":78},"updated":"2017-03-29 20:06:07.000000000","message":"I find this docstring a little mis-leading. It doesn\u0027t solve conflicts, it raises an exception if conflicts are discovered. We don\u0027t check either \"various QoS policies\". We check that a new rule doesn\u0027t conflict with the existing rules in the policy. So a better version might be:\n\nThis function checks that a new rule doesn\u0027t conflict with the existing rules in the policy. If a conflict is discovered, an exception is returned to the API.","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"bcd1acef29feacfd39558ba3ee6e96b36444e034","unresolved":false,"context_lines":[{"line_number":33,"context_line":"            if \"max_kbps\" in rule_data and ("},{"line_number":34,"context_line":"               int(rule.min_kbps) \u003e int(rule_data[\"max_kbps\"])):"},{"line_number":35,"context_line":"                    raise n_exc.QoSRuleParameterConflict("},{"line_number":36,"context_line":"                        rule_value\u003drule_data[\"max_kbps\"],"},{"line_number":37,"context_line":"                        policy_id\u003dpolicy[\u0027id\u0027],"},{"line_number":38,"context_line":"                        existing_rule\u003drule.rule_type,"},{"line_number":39,"context_line":"                        existing_value\u003drule.min_kbps)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bff0334d_3cd5df1c","line":36,"range":{"start_line":36,"start_character":45,"end_line":36,"end_character":55},"updated":"2017-04-10 12:11:47.000000000","message":"nit - can we be consistent with the \u0027 and \". below you use \u0027","commit_id":"839e02380d9dbb8408f7b262490bd1fa2350ded5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7294c5ee07910fe7927631b203edbebab0de9dc2","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    \"\"\"Implementation of the QoS Rule checker."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    This function checks if the new rule to be associated with the policy"},{"line_number":25,"context_line":"    doesnt conflict with the existing rules."},{"line_number":26,"context_line":"    Raises an exception if conflict is identified."},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":"    for rule in policy.rules:"}],"source_content_type":"text/x-python","patch_set":18,"id":"bff0334d_96539539","line":25,"updated":"2017-04-10 21:09:13.000000000","message":"nit: probably should be doesn\u0027t","commit_id":"ff6dcf841313063da0f5d05875a760dc46f7dac2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ca08141c433d3d39f7451ad932aa99fc1f20e60e","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        if rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_DSCP_MARKING:"},{"line_number":30,"context_line":"            # Skip checks if Rule is DSCP"},{"line_number":31,"context_line":"            continue"},{"line_number":32,"context_line":"        if rule.rule_type \u003d\u003d qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH:"},{"line_number":33,"context_line":"            if \"max_kbps\" in rule_data and ("},{"line_number":34,"context_line":"               int(rule.min_kbps) \u003e int(rule_data[\"max_kbps\"])):"},{"line_number":35,"context_line":"                    raise n_exc.QoSRuleParameterConflict("}],"source_content_type":"text/x-python","patch_set":18,"id":"bff0334d_6a521d15","line":32,"range":{"start_line":32,"start_character":8,"end_line":32,"end_character":10},"updated":"2017-04-12 16:11:22.000000000","message":"nit: elif","commit_id":"ff6dcf841313063da0f5d05875a760dc46f7dac2"}],"neutron/services/qos/qos_plugin.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ff9fed929b0f2554fd58ff6715469396bfc37ddb","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        with db_api.autonested_transaction(context.session):"},{"line_number":201,"context_line":"            # Ensure that we have access to the policy."},{"line_number":202,"context_line":"            policy \u003d self._get_policy_obj(context, policy_id)"},{"line_number":203,"context_line":"            # if no rule is defined for the policy, the for loop wont execute"},{"line_number":204,"context_line":"            for rule in policy.rules:"},{"line_number":205,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":206,"context_line":"                    if \"max_kbps\" in rule_data and ("}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_02b62ddc","line":203,"updated":"2017-03-07 10:57:27.000000000","message":"Now you are checking only if you are creating a BW limit rule. You should check also if you are creating a Min BW rule:\n\nif new rule \u003d\u003d bw_limit -\u003e check min_bw\nif new rule \u003d\u003d min_bw --\u003e check bw_limit","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"923883e92bf7e024bce53d6e2b5e6f04f72e0f30","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        with db_api.autonested_transaction(context.session):"},{"line_number":201,"context_line":"            # Ensure that we have access to the policy."},{"line_number":202,"context_line":"            policy \u003d self._get_policy_obj(context, policy_id)"},{"line_number":203,"context_line":"            # if no rule is defined for the policy, the for loop wont execute"},{"line_number":204,"context_line":"            for rule in policy.rules:"},{"line_number":205,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":206,"context_line":"                    if \"max_kbps\" in rule_data and ("}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_d6b6933c","line":203,"in_reply_to":"9a30ddce_02b62ddc","updated":"2017-03-07 12:23:41.000000000","message":"Done","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ff9fed929b0f2554fd58ff6715469396bfc37ddb","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            policy \u003d self._get_policy_obj(context, policy_id)"},{"line_number":203,"context_line":"            # if no rule is defined for the policy, the for loop wont execute"},{"line_number":204,"context_line":"            for rule in policy.rules:"},{"line_number":205,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":206,"context_line":"                    if \"max_kbps\" in rule_data and ("},{"line_number":207,"context_line":"                       int(rule.min_kbps) \u003c int(rule_data[\"max_kbps\"])):"},{"line_number":208,"context_line":"                       raise n_exc.QoSRuleParameterConflict("}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_02a1cda9","line":205,"range":{"start_line":205,"start_character":19,"end_line":205,"end_character":38},"updated":"2017-03-07 10:57:27.000000000","message":"You can read the type of rule by reading QoSRule.rule_type","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"923883e92bf7e024bce53d6e2b5e6f04f72e0f30","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            policy \u003d self._get_policy_obj(context, policy_id)"},{"line_number":203,"context_line":"            # if no rule is defined for the policy, the for loop wont execute"},{"line_number":204,"context_line":"            for rule in policy.rules:"},{"line_number":205,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":206,"context_line":"                    if \"max_kbps\" in rule_data and ("},{"line_number":207,"context_line":"                       int(rule.min_kbps) \u003c int(rule_data[\"max_kbps\"])):"},{"line_number":208,"context_line":"                       raise n_exc.QoSRuleParameterConflict("}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_9b8604b8","line":205,"range":{"start_line":205,"start_character":19,"end_line":205,"end_character":38},"in_reply_to":"9a30ddce_02a1cda9","updated":"2017-03-07 12:23:41.000000000","message":"Does it make a difference ?","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"aa8b23f85fc7f6f958d52b6f995f2b0cb364ed7e","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            policy \u003d self._get_policy_obj(context, policy_id)"},{"line_number":203,"context_line":"            # if no rule is defined for the policy, the for loop wont execute"},{"line_number":204,"context_line":"            for rule in policy.rules:"},{"line_number":205,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":206,"context_line":"                    if \"max_kbps\" in rule_data and ("},{"line_number":207,"context_line":"                       int(rule.min_kbps) \u003c int(rule_data[\"max_kbps\"])):"},{"line_number":208,"context_line":"                       raise n_exc.QoSRuleParameterConflict("}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_f149e18f","line":205,"range":{"start_line":205,"start_character":19,"end_line":205,"end_character":38},"in_reply_to":"9a30ddce_9b8604b8","updated":"2017-03-07 12:45:32.000000000","message":"Well, it doesn\u0027t make any difference, but I think is better to compare QoSRule.rule_type to neutron.services.qos.qos_consts:\n RULE_TYPE_BANDWIDTH_LIMIT \u003d \u0027bandwidth_limit\u0027\n RULE_TYPE_DSCP_MARKING \u003d \u0027dscp_marking\u0027\n RULE_TYPE_MINIMUM_BANDWIDTH \u003d \u0027minimum_bandwidth\u0027","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":18051,"name":"David Shaughnessy","email":"david.shaughnessy@intel.com","username":"davidsha"},"change_message_id":"6b9f5379f210455da288e44328e259fc94e501ac","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            for rule in policy.rules:"},{"line_number":205,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":206,"context_line":"                    if \"max_kbps\" in rule_data and ("},{"line_number":207,"context_line":"                       int(rule.min_kbps) \u003c int(rule_data[\"max_kbps\"])):"},{"line_number":208,"context_line":"                       raise n_exc.QoSRuleParameterConflict("},{"line_number":209,"context_line":"                           policy_id\u003dpolicy_id,"},{"line_number":210,"context_line":"                           min_kbps\u003drule.min_kbps,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_e273d94b","line":207,"range":{"start_line":207,"start_character":48,"end_line":207,"end_character":69},"updated":"2017-03-07 10:59:01.000000000","message":"Is this assuming that you\u0027ll add the min bandwidth rule before you add the bandwidth limit?","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"923883e92bf7e024bce53d6e2b5e6f04f72e0f30","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            for rule in policy.rules:"},{"line_number":205,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":206,"context_line":"                    if \"max_kbps\" in rule_data and ("},{"line_number":207,"context_line":"                       int(rule.min_kbps) \u003c int(rule_data[\"max_kbps\"])):"},{"line_number":208,"context_line":"                       raise n_exc.QoSRuleParameterConflict("},{"line_number":209,"context_line":"                           policy_id\u003dpolicy_id,"},{"line_number":210,"context_line":"                           min_kbps\u003drule.min_kbps,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_7696c795","line":207,"range":{"start_line":207,"start_character":48,"end_line":207,"end_character":69},"in_reply_to":"9a30ddce_e273d94b","updated":"2017-03-07 12:23:41.000000000","message":"Yes, but Rodolfo\u0027s comment has been handled now , and now it checks for BW Limit rule and Min BW Rule","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ff9fed929b0f2554fd58ff6715469396bfc37ddb","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        return rule"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    @db_base_plugin_common.convert_result_to_dict"},{"line_number":224,"context_line":"    def update_policy_rule(self, context, rule_cls, rule_id, policy_id,"},{"line_number":225,"context_line":"            rule_data):"},{"line_number":226,"context_line":"        \"\"\"Update a QoS policy rule."},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_e2c11930","line":224,"updated":"2017-03-07 10:57:27.000000000","message":"You should check also when a rule is updated.","commit_id":"c12af35524b430543292efe039d6d5e219881ddf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"73fd0f5c6ee74b13d17fb31aeca202422378bd8a","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            pager \u003d base_obj.Pager(sorts, limit, page_reverse, marker)"},{"line_number":329,"context_line":"            return rule_cls.get_objects(context, _pager\u003dpager, **filters)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"    def _check_rule_conflict(self, policy, rule_data):"},{"line_number":332,"context_line":"        # if no rule is defined for the policy, the for loop wont execute"},{"line_number":333,"context_line":"            for rule in policy.rules:"},{"line_number":334,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_c7a7bf4d","line":331,"updated":"2017-03-07 13:18:48.000000000","message":"I would move this rule conflict checking to a separate class where we can add the logic for rule compatibility (which over time is going to be more and more complicated),\n\nand may be rules/policies could check semantics before updating/storing-to database with such class, in a way that we don\u0027t need to grow the qos_plugin.py codebase.","commit_id":"ec19a04511d27ed444b34acad3587daacf1d61e3"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"73fd0f5c6ee74b13d17fb31aeca202422378bd8a","unresolved":false,"context_lines":[{"line_number":331,"context_line":"    def _check_rule_conflict(self, policy, rule_data):"},{"line_number":332,"context_line":"        # if no rule is defined for the policy, the for loop wont execute"},{"line_number":333,"context_line":"            for rule in policy.rules:"},{"line_number":334,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":335,"context_line":"                    if \"max_kbps\" in rule_data and ("},{"line_number":336,"context_line":"                       int(rule.min_kbps) \u003e int(rule_data[\"max_kbps\"])):"},{"line_number":337,"context_line":"                       raise n_exc.QoSRuleParameterConflict("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_4772efe6","line":334,"updated":"2017-03-07 13:18:48.000000000","message":"please use \"isinstance\" instead :)","commit_id":"ec19a04511d27ed444b34acad3587daacf1d61e3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1e4d83b5ae95b3dcd5d34c1d46ec093c503901cb","unresolved":false,"context_lines":[{"line_number":331,"context_line":"    def _check_rule_conflict(self, policy, rule_data):"},{"line_number":332,"context_line":"        # if no rule is defined for the policy, the for loop wont execute"},{"line_number":333,"context_line":"            for rule in policy.rules:"},{"line_number":334,"context_line":"                if type(rule).__name__ \u003d\u003d \"QosMinimumBandwidthRule\":"},{"line_number":335,"context_line":"                    if \"max_kbps\" in rule_data and ("},{"line_number":336,"context_line":"                       int(rule.min_kbps) \u003e int(rule_data[\"max_kbps\"])):"},{"line_number":337,"context_line":"                       raise n_exc.QoSRuleParameterConflict("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_d5e2f602","line":334,"in_reply_to":"9a30ddce_4772efe6","updated":"2017-03-07 17:58:08.000000000","message":"Sorry for insist again on this but you should use QoSRule.rule_type and compare to neutron.services.qos.qos_consts\n  .RULE_TYPE_BANDWIDTH_LIMIT \u003d \u0027bandwidth_limit\u0027\n  .RULE_TYPE_DSCP_MARKING \u003d \u0027dscp_marking\u0027\n  .RULE_TYPE_MINIMUM_BANDWIDTH \u003d \u0027minimum_bandwidth\u0027","commit_id":"ec19a04511d27ed444b34acad3587daacf1d61e3"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"72421140e2eb13661e82299834ce146c982a4504","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        with db_api.autonested_transaction(context.session):"},{"line_number":202,"context_line":"            # Ensure that we have access to the policy."},{"line_number":203,"context_line":"            policy \u003d self._get_policy_obj(context, policy_id)"},{"line_number":204,"context_line":"            checker.ObjectChecker.check_rule_conflict(policy, rule_data)"},{"line_number":205,"context_line":"            rule \u003d rule_cls(context, qos_policy_id\u003dpolicy_id, **rule_data)"},{"line_number":206,"context_line":"            rule.create()"},{"line_number":207,"context_line":"            policy.reload_rules()"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_f09c445f","line":204,"updated":"2017-03-08 12:17:21.000000000","message":"Ok, now looks better! And as Reedip and Miguel Angel said, could be just a function.","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"5f9053952929ea5c6e3c3fe81a515e410cc1588d","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        with db_api.autonested_transaction(context.session):"},{"line_number":202,"context_line":"            # Ensure that we have access to the policy."},{"line_number":203,"context_line":"            policy \u003d self._get_policy_obj(context, policy_id)"},{"line_number":204,"context_line":"            checker.ObjectChecker.check_rule_conflict(policy, rule_data)"},{"line_number":205,"context_line":"            rule \u003d rule_cls(context, qos_policy_id\u003dpolicy_id, **rule_data)"},{"line_number":206,"context_line":"            rule.create()"},{"line_number":207,"context_line":"            policy.reload_rules()"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_c6fa0ebd","line":204,"in_reply_to":"9a30ddce_f09c445f","updated":"2017-03-09 04:19:18.000000000","message":"Done","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"72421140e2eb13661e82299834ce146c982a4504","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            filters[qos_consts.QOS_POLICY_ID] \u003d policy_id"},{"line_number":329,"context_line":"            pager \u003d base_obj.Pager(sorts, limit, page_reverse, marker)"},{"line_number":330,"context_line":"            return rule_cls.get_objects(context, _pager\u003dpager, **filters)"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a30ddce_30917c53","line":331,"updated":"2017-03-08 12:17:21.000000000","message":"nit: remove this line","commit_id":"8416f48cb6f6b7b64be62633afa1c21301bd551e"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"20a3132a234c48bc631bde10f86aa27fa3a5402c","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.db import db_base_plugin_common"},{"line_number":19,"context_line":"from neutron.extensions import qos"},{"line_number":20,"context_line":"from neutron.objects import base as base_obj"},{"line_number":21,"context_line":"from neutron.objects.qos import qos_policy_validator as checker"},{"line_number":22,"context_line":"from neutron.objects.qos import policy as policy_object"},{"line_number":23,"context_line":"from neutron.objects.qos import rule_type as rule_type_object"},{"line_number":24,"context_line":"from neutron.services.qos.drivers import manager"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_d7ec37bc","line":21,"updated":"2017-03-29 20:06:07.000000000","message":"This has to go after line 22","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"}],"neutron/tests/unit/services/qos/test_qos_plugin.py":[{"author":{"_account_id":15309,"name":"zhaobo","email":"bzhaojyathousandy@gmail.com","username":"ZhaoBo"},"change_message_id":"b11e1bbfbcdafe873894e483c3817c221f98f2e9","unresolved":false,"context_lines":[{"line_number":175,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":176,"context_line":"            self.rule_data[\u0027min_bw_rule\u0027] \u003d {\u0027id\u0027: uuidutils.generate_uuid(),"},{"line_number":177,"context_line":"                           \u0027min_kbps\u0027: 100}"},{"line_number":178,"context_line":"            self.assertRasies("},{"line_number":179,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":180,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":181,"context_line":"                self.ctxt, self.rule_data[\u0027min_bw_rule\u0027].id, self.policy.id,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a1ced50_59b466d0","line":178,"range":{"start_line":178,"start_character":17,"end_line":178,"end_character":29},"updated":"2017-03-20 06:57:51.000000000","message":"nic....:)","commit_id":"48629211fe5a0699a04c239e9db154c2eb72a28d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2b20eac1fa01855693dc3bd617ee8521c3bfb93e","unresolved":false,"context_lines":[{"line_number":173,"context_line":"            self.qos_plugin.update_policy_bandwidth_limit_rule("},{"line_number":174,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":175,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":176,"context_line":"            self.rule_data[\u0027min_bw_rule\u0027] \u003d {\u0027id\u0027: uuidutils.generate_uuid(),"},{"line_number":177,"context_line":"                           \u0027min_kbps\u0027: 100}"},{"line_number":178,"context_line":"            self.assertRaises("},{"line_number":179,"context_line":"                n_exc.QoSRuleParameterConflict,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a1ced50_9204f07e","line":176,"range":{"start_line":176,"start_character":28,"end_line":176,"end_character":39},"updated":"2017-03-20 11:11:42.000000000","message":"this should be \"minimum_bandwidth_rule\"","commit_id":"453f01cd731877e48b972d4b02cb2093ba810586"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5a6a3d3c053c5bd81a45b6e4747c18b4b59af6d7","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            self.assertRaises("},{"line_number":180,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":181,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":182,"context_line":"                self.ctxt, self.rule_data[\u0027min_bw_rule\u0027][\u0027id\u0027],"},{"line_number":183,"context_line":"                self.policy.id, self.rule_data)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_update_policy_rule_bad_policy(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a1ced50_b166b2b8","line":182,"range":{"start_line":182,"start_character":43,"end_line":182,"end_character":54},"updated":"2017-03-23 09:42:44.000000000","message":"Hmmm the same mistake hehehe (must be minimum_bandwidth_rule)","commit_id":"68d551e1b2e18ea614440f1c6fda25758dcca454"},{"author":{"_account_id":18051,"name":"David Shaughnessy","email":"david.shaughnessy@intel.com","username":"davidsha"},"change_message_id":"f7f8b8eec07c9abe5352700617d679e09cc14431","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            self.rule_data[\u0027minimum_bandwidth_rule\u0027] \u003d {"},{"line_number":177,"context_line":"                \u0027id\u0027: uuidutils.generate_uuid(),"},{"line_number":178,"context_line":"                \u0027min_kbps\u0027: 100}"},{"line_number":179,"context_line":"            self.assertRaises("},{"line_number":180,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":181,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":182,"context_line":"                self.ctxt, self.rule_data[\u0027minimum_bandwidth_rule\u0027][\u0027id\u0027],"},{"line_number":183,"context_line":"                self.policy.id, self.rule_data)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_update_policy_rule_bad_policy(self):"},{"line_number":186,"context_line":"        _policy \u003d policy_object.QosPolicy("}],"source_content_type":"text/x-python","patch_set":11,"id":"1a1ced50_5d057b12","line":183,"range":{"start_line":179,"start_character":12,"end_line":183,"end_character":47},"updated":"2017-03-23 17:18:01.000000000","message":"This seems to be failing because it\u0027s trying to make a database transaction by retrieving the qos_rule object using the policy_id and rule_id.\n\nhttp://logs.openstack.org/75/442375/11/check/gate-neutron-python27-ubuntu-xenial/3c5b2de/console.html#_2017-03-23_12_51_00_066774\n\nYou need to mock the call and return a dummy object I think.","commit_id":"a665aded93391770c7685fa289e1cbcc08e896b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c134eaf158537d2b106232056eaedf412c44938f","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            self.rule_data[\u0027minimum_bandwidth_rule\u0027] \u003d {"},{"line_number":177,"context_line":"                \u0027id\u0027: uuidutils.generate_uuid(),"},{"line_number":178,"context_line":"                \u0027min_kbps\u0027: 100}"},{"line_number":179,"context_line":"            self.assertRaises("},{"line_number":180,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":181,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":182,"context_line":"                self.ctxt, self.rule_data[\u0027minimum_bandwidth_rule\u0027][\u0027id\u0027],"},{"line_number":183,"context_line":"                self.policy.id, self.rule_data)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_update_policy_rule_bad_policy(self):"},{"line_number":186,"context_line":"        _policy \u003d policy_object.QosPolicy("}],"source_content_type":"text/x-python","patch_set":11,"id":"1a1ced50_cc08dedc","line":183,"range":{"start_line":179,"start_character":12,"end_line":183,"end_character":47},"in_reply_to":"1a1ced50_5d057b12","updated":"2017-03-23 17:46:16.000000000","message":"No, the problem is self.rule is a MAX_BW rule --\u003e this QoS policy has only one rule of MAX_BW. \"update_policy_minimum_bandwidth_rule\" won\u0027t get any MIN_BW rule.\n\nPlease, in this test update the same type of rule (bandwidth_limit_rule) with min_kbps.","commit_id":"a665aded93391770c7685fa289e1cbcc08e896b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7f14e83736d84ce01d6279f58181ded01c8352b0","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            self.rule_data[\u0027minimum_bandwidth_rule\u0027] \u003d {"},{"line_number":177,"context_line":"                \u0027id\u0027: uuidutils.generate_uuid(),"},{"line_number":178,"context_line":"                \u0027min_kbps\u0027: 100}"},{"line_number":179,"context_line":"            self.assertRaises("},{"line_number":180,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":181,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":182,"context_line":"                self.ctxt, self.rule_data[\u0027minimum_bandwidth_rule\u0027][\u0027id\u0027],"},{"line_number":183,"context_line":"                self.policy.id, self.rule_data)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def test_update_policy_rule_bad_policy(self):"},{"line_number":186,"context_line":"        _policy \u003d policy_object.QosPolicy("}],"source_content_type":"text/x-python","patch_set":11,"id":"ffe62b97_d661b63b","line":183,"range":{"start_line":179,"start_character":12,"end_line":183,"end_character":47},"in_reply_to":"1a1ced50_cc08dedc","updated":"2017-03-24 07:30:54.000000000","message":"I\u0027m going to amend my comment.\n\nWhat you need here is to create a QoS policy with two rules (min and max). Then you need to:\n- Change one (min) satisfying the limits.\n- Change one (min) no satisfying the limits.\n- Change one (max) satisfying the limits.\n- Change one (max) no doing this.\n\nAlso you should check the limits during the cration process:\n- QoS policy with one rule (max); then you create a min rule exceeding the limits.\n- QoS policy with one min rule; add a max rule exceeding the limits.","commit_id":"a665aded93391770c7685fa289e1cbcc08e896b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"59d3bb30a5661eb9a382a918c9d55f434e73ccc9","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        self.qos_plugin.delete_policy(self.ctxt, self.policy.id)"},{"line_number":151,"context_line":"        self._validate_driver_params(\u0027delete_policy\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @mock.patch(\u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":154,"context_line":"                \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone)"},{"line_number":155,"context_line":"    def test_create_policy_rule(self, *mocks):"},{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy):"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_2faf952a","line":154,"range":{"start_line":153,"start_character":4,"end_line":154,"end_character":68},"updated":"2017-03-29 16:32:20.000000000","message":"Intead of having a decorator and a \"with\" clause in this function, you can join both inside the with clause.","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        self.qos_plugin.delete_policy(self.ctxt, self.policy.id)"},{"line_number":151,"context_line":"        self._validate_driver_params(\u0027delete_policy\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @mock.patch(\u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":154,"context_line":"                \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone)"},{"line_number":155,"context_line":"    def test_create_policy_rule(self, *mocks):"},{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy):"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_c36112d5","line":154,"range":{"start_line":153,"start_character":4,"end_line":154,"end_character":68},"in_reply_to":"ffe62b97_2faf952a","updated":"2017-03-30 00:43:02.000000000","message":"Fixed","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"59d3bb30a5661eb9a382a918c9d55f434e73ccc9","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def test_update_policy_rule_check_rule_min_less_than_max(self):"},{"line_number":173,"context_line":"        _policy \u003d self.get_policy()"},{"line_number":174,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":175,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":176,"context_line":"                        return_value\u003d[self.rule]), mock.patch("},{"line_number":177,"context_line":"            \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_fa2e0d8a","line":174,"range":{"start_line":174,"start_character":8,"end_line":174,"end_character":46},"updated":"2017-03-29 16:32:20.000000000","message":"You should add both rules (max and min) at the beginning","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"41614239f143dc789366bb8a866dad895af64aa2","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def test_update_policy_rule_check_rule_min_less_than_max(self):"},{"line_number":173,"context_line":"        _policy \u003d self.get_policy()"},{"line_number":174,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":175,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":176,"context_line":"                        return_value\u003d[self.rule]), mock.patch("},{"line_number":177,"context_line":"            \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_48b4f3f2","line":174,"range":{"start_line":174,"start_character":8,"end_line":174,"end_character":46},"in_reply_to":"ffe62b97_036c9acb","updated":"2017-03-30 09:24:47.000000000","message":"Yes, but not in this case: you don\u0027t have a conflicting rule here.\n\nAnyway, this comment is almost a nit.","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def test_update_policy_rule_check_rule_min_less_than_max(self):"},{"line_number":173,"context_line":"        _policy \u003d self.get_policy()"},{"line_number":174,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":175,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":176,"context_line":"                        return_value\u003d[self.rule]), mock.patch("},{"line_number":177,"context_line":"            \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_036c9acb","line":174,"range":{"start_line":174,"start_character":8,"end_line":174,"end_character":46},"in_reply_to":"ffe62b97_fa2e0d8a","updated":"2017-03-30 00:43:02.000000000","message":"I wanted to simulate the behavior when a policy is created blank, then one rule is updated, and then the sceond conflicting rule is provided.","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"59d3bb30a5661eb9a382a918c9d55f434e73ccc9","unresolved":false,"context_lines":[{"line_number":181,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":182,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        rules \u003d [self.rule, self.min_rule]"},{"line_number":185,"context_line":"        setattr(_policy, \"rules\", rules)"},{"line_number":186,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":187,"context_line":"                        return_value\u003drules),mock.patch("}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_ba52c511","line":184,"range":{"start_line":184,"start_character":8,"end_line":184,"end_character":42},"updated":"2017-03-29 16:32:20.000000000","message":"ditto","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":181,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":182,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        rules \u003d [self.rule, self.min_rule]"},{"line_number":185,"context_line":"        setattr(_policy, \"rules\", rules)"},{"line_number":186,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":187,"context_line":"                        return_value\u003drules),mock.patch("}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_839d4abd","line":184,"range":{"start_line":184,"start_character":8,"end_line":184,"end_character":42},"in_reply_to":"ffe62b97_ba52c511","updated":"2017-03-30 00:43:02.000000000","message":"ditto","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"20a3132a234c48bc631bde10f86aa27fa3a5402c","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                            return_value\u003d_policy):"},{"line_number":190,"context_line":"            self.qos_plugin.update_policy_minimum_bandwidth_rule("},{"line_number":191,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":192,"context_line":"                self.policy.id,self.rule_data)"},{"line_number":193,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":194,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_97a6afc0","line":192,"range":{"start_line":192,"start_character":30,"end_line":192,"end_character":31},"updated":"2017-03-29 20:06:07.000000000","message":"missing space after the comma","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                            return_value\u003d_policy):"},{"line_number":190,"context_line":"            self.qos_plugin.update_policy_minimum_bandwidth_rule("},{"line_number":191,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":192,"context_line":"                self.policy.id,self.rule_data)"},{"line_number":193,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":194,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_a38a8e60","line":192,"range":{"start_line":192,"start_character":30,"end_line":192,"end_character":31},"in_reply_to":"ffe62b97_97a6afc0","updated":"2017-03-30 00:43:02.000000000","message":"Done","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"59d3bb30a5661eb9a382a918c9d55f434e73ccc9","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":205,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":206,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":207,"context_line":"        import copy"},{"line_number":208,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":209,"context_line":"        data[\u0027minimum_bandwidth_rule\u0027][\u0027min_kbps\u0027] \u003d 1000"},{"line_number":210,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_efe9ade2","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":19},"updated":"2017-03-29 16:32:20.000000000","message":"Move this up, to the imports section","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":205,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":206,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":207,"context_line":"        import copy"},{"line_number":208,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":209,"context_line":"        data[\u0027minimum_bandwidth_rule\u0027][\u0027min_kbps\u0027] \u003d 1000"},{"line_number":210,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_83eb0a11","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":19},"in_reply_to":"ffe62b97_efe9ade2","updated":"2017-03-30 00:43:02.000000000","message":"Done","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"59d3bb30a5661eb9a382a918c9d55f434e73ccc9","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                        return_value\u003d_policy):"},{"line_number":212,"context_line":"            self.assertRaises("},{"line_number":213,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":214,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":215,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":216,"context_line":"                self.policy.id,data)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_ba6525b7","line":214,"range":{"start_line":214,"start_character":16,"end_line":214,"end_character":69},"updated":"2017-03-29 16:32:20.000000000","message":"You shouldn\u0027t update a min rule in this QoS policy because doesn\u0027t have it.\n\nThis should throw exceptions.QosRuleNotFound","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"41614239f143dc789366bb8a866dad895af64aa2","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                        return_value\u003d_policy):"},{"line_number":212,"context_line":"            self.assertRaises("},{"line_number":213,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":214,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":215,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":216,"context_line":"                self.policy.id,data)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_485ab383","line":214,"range":{"start_line":214,"start_character":16,"end_line":214,"end_character":69},"in_reply_to":"ffe62b97_039a5ab7","updated":"2017-03-30 09:24:47.000000000","message":"Ok, the rule is tested in advance and the test is passing.","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"20a3132a234c48bc631bde10f86aa27fa3a5402c","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                        return_value\u003d_policy):"},{"line_number":212,"context_line":"            self.assertRaises("},{"line_number":213,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":214,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":215,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":216,"context_line":"                self.policy.id,data)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_d7fb17bb","line":214,"range":{"start_line":214,"start_character":16,"end_line":214,"end_character":69},"in_reply_to":"ffe62b97_ba6525b7","updated":"2017-03-29 20:06:07.000000000","message":"I maybe be misunderstanding Rodolfo\u0027s comment but the test case passed:\n\n2017-03-29 16:10:19.205063 | {3} neutron.tests.unit.services.qos.test_qos_plugin.TestQosPlugin.test_update_policy_rule_check_rule_bwlimit_less_than_minbw [0.077214s] ... ok","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                        return_value\u003d_policy):"},{"line_number":212,"context_line":"            self.assertRaises("},{"line_number":213,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":214,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":215,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":216,"context_line":"                self.policy.id,data)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_039a5ab7","line":214,"range":{"start_line":214,"start_character":16,"end_line":214,"end_character":69},"in_reply_to":"ffe62b97_ba6525b7","updated":"2017-03-30 00:43:02.000000000","message":"It wont, because it fails the check of minimum bw \u003e  max kbps specified by bw limit rule","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"59d3bb30a5661eb9a382a918c9d55f434e73ccc9","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":214,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":215,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":216,"context_line":"                self.policy.id,data)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def test_update_policy_rule_check_rule_minbw_gr_than_bwlimit(self):"},{"line_number":219,"context_line":"        _policy \u003d self.get_policy()"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_3a711577","line":216,"updated":"2017-03-29 16:32:20.000000000","message":"add space","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":214,"context_line":"                self.qos_plugin.update_policy_minimum_bandwidth_rule,"},{"line_number":215,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":216,"context_line":"                self.policy.id,data)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def test_update_policy_rule_check_rule_minbw_gr_than_bwlimit(self):"},{"line_number":219,"context_line":"        _policy \u003d self.get_policy()"}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_03dffa68","line":216,"in_reply_to":"ffe62b97_3a711577","updated":"2017-03-30 00:43:02.000000000","message":"Done","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"59d3bb30a5661eb9a382a918c9d55f434e73ccc9","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                        return_value\u003d_policy):"},{"line_number":234,"context_line":"            self.assertRaises("},{"line_number":235,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":236,"context_line":"                self.qos_plugin.update_policy_bandwidth_limit_rule,"},{"line_number":237,"context_line":"                self.ctxt, self.rule.id,"},{"line_number":238,"context_line":"                self.policy.id,data)"},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_9a0dc1e9","line":236,"range":{"start_line":236,"start_character":32,"end_line":236,"end_character":66},"updated":"2017-03-29 16:32:20.000000000","message":"ditto, there is no max rule in this policy","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"20a3132a234c48bc631bde10f86aa27fa3a5402c","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                        return_value\u003d_policy):"},{"line_number":234,"context_line":"            self.assertRaises("},{"line_number":235,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":236,"context_line":"                self.qos_plugin.update_policy_bandwidth_limit_rule,"},{"line_number":237,"context_line":"                self.ctxt, self.rule.id,"},{"line_number":238,"context_line":"                self.policy.id,data)"},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_37cec362","line":236,"range":{"start_line":236,"start_character":32,"end_line":236,"end_character":66},"in_reply_to":"ffe62b97_9a0dc1e9","updated":"2017-03-29 20:06:07.000000000","message":"Again, I maybe misunderstanding the comment, but the test case passed:\n\nneutron.tests.unit.services.qos.test_qos_plugin.TestQosPlugin.test_update_policy_rule_check_rule_minbw_gr_than_bwlimit [0.081605s] ... ok","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":233,"context_line":"                        return_value\u003d_policy):"},{"line_number":234,"context_line":"            self.assertRaises("},{"line_number":235,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":236,"context_line":"                self.qos_plugin.update_policy_bandwidth_limit_rule,"},{"line_number":237,"context_line":"                self.ctxt, self.rule.id,"},{"line_number":238,"context_line":"                self.policy.id,data)"},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_433f4286","line":236,"range":{"start_line":236,"start_character":32,"end_line":236,"end_character":66},"in_reply_to":"ffe62b97_9a0dc1e9","updated":"2017-03-30 00:43:02.000000000","message":"We are going to update the qos_policy with bandwidth limit rule. but the update fails because of the check I introduced for checking the bandwidth values","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"20a3132a234c48bc631bde10f86aa27fa3a5402c","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":236,"context_line":"                self.qos_plugin.update_policy_bandwidth_limit_rule,"},{"line_number":237,"context_line":"                self.ctxt, self.rule.id,"},{"line_number":238,"context_line":"                self.policy.id,data)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def get_policy(self):"},{"line_number":241,"context_line":"        return policy_object.QosPolicy("}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_37d36306","line":238,"range":{"start_line":238,"start_character":30,"end_line":238,"end_character":31},"updated":"2017-03-29 20:06:07.000000000","message":"Missing space after comma","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                n_exc.QoSRuleParameterConflict,"},{"line_number":236,"context_line":"                self.qos_plugin.update_policy_bandwidth_limit_rule,"},{"line_number":237,"context_line":"                self.ctxt, self.rule.id,"},{"line_number":238,"context_line":"                self.policy.id,data)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def get_policy(self):"},{"line_number":241,"context_line":"        return policy_object.QosPolicy("}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_2342fe0f","line":238,"range":{"start_line":238,"start_character":30,"end_line":238,"end_character":31},"in_reply_to":"ffe62b97_37d36306","updated":"2017-03-30 00:43:02.000000000","message":"Done","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"20a3132a234c48bc631bde10f86aa27fa3a5402c","unresolved":false,"context_lines":[{"line_number":237,"context_line":"                self.ctxt, self.rule.id,"},{"line_number":238,"context_line":"                self.policy.id,data)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def get_policy(self):"},{"line_number":241,"context_line":"        return policy_object.QosPolicy("},{"line_number":242,"context_line":"            self.ctxt, **self.policy_data[\u0027policy\u0027])"},{"line_number":243,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_4143c542","line":240,"range":{"start_line":240,"start_character":8,"end_line":240,"end_character":18},"updated":"2017-03-29 20:06:07.000000000","message":"This should be _get_policy(","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"db15c21bd8a806a31d1d2a2add1c6c7c406f2e23","unresolved":false,"context_lines":[{"line_number":237,"context_line":"                self.ctxt, self.rule.id,"},{"line_number":238,"context_line":"                self.policy.id,data)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def get_policy(self):"},{"line_number":241,"context_line":"        return policy_object.QosPolicy("},{"line_number":242,"context_line":"            self.ctxt, **self.policy_data[\u0027policy\u0027])"},{"line_number":243,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"ffe62b97_034dba3c","line":240,"range":{"start_line":240,"start_character":8,"end_line":240,"end_character":18},"in_reply_to":"ffe62b97_4143c542","updated":"2017-03-30 00:43:02.000000000","message":"Done","commit_id":"aaba3936f46ea71f857f658c24c25f1cc9bd4230"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7294c5ee07910fe7927631b203edbebab0de9dc2","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":159,"context_line":"            \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone):"},{"line_number":160,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":161,"context_line":"                self.ctxt, self.policy.id, self.rule_data)"},{"line_number":162,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bff0334d_16166556","line":159,"updated":"2017-04-10 21:09:13.000000000","message":"nit: this\n\nreturn_value\u003dNone\n\nis not necessary here","commit_id":"ff6dcf841313063da0f5d05875a760dc46f7dac2"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"43ffb520a0b72dd75c5f10629bf08fcd295aa3f5","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":159,"context_line":"            \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone):"},{"line_number":160,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":161,"context_line":"                self.ctxt, self.policy.id, self.rule_data)"},{"line_number":162,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bff0334d_7dbfcc60","line":159,"in_reply_to":"bff0334d_16166556","updated":"2017-04-11 01:22:04.000000000","message":"I would like only none to be returned to ensure the function doesn\u0027t do anything weird :)","commit_id":"ff6dcf841313063da0f5d05875a760dc46f7dac2"},{"author":{"_account_id":20378,"name":"Trevor McCasland","email":"TM2086@att.com","username":"twm2016"},"change_message_id":"edf44cf70beb86973c2f88dc499ab002082c7789","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        self.qos_plugin.delete_policy(self.ctxt, self.policy.id)"},{"line_number":153,"context_line":"        self._validate_driver_params(\u0027delete_policy\u0027)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def test_create_policy_rule(self, *mocks):"},{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"bff0334d_c5a91a24","line":155,"range":{"start_line":155,"start_character":38,"end_line":155,"end_character":44},"updated":"2017-04-12 17:23:23.000000000","message":"I think this isn\u0027t necessary. *mocks is needed on L151 because of the use of the mock decorator.","commit_id":"ef50fa5b715b4dcedb5c59faabc9dedfc1f6bcb6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4353c231f2e8d3270c66a0fede77de6931e9dd10","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        self.qos_plugin.delete_policy(self.ctxt, self.policy.id)"},{"line_number":153,"context_line":"        self._validate_driver_params(\u0027delete_policy\u0027)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def test_create_policy_rule(self, *mocks):"},{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"9ff52f3f_61dc6d46","line":155,"range":{"start_line":155,"start_character":38,"end_line":155,"end_character":44},"in_reply_to":"bff0334d_c5a91a24","updated":"2017-04-13 08:26:16.000000000","message":"Good catch Trevor","commit_id":"ef50fa5b715b4dcedb5c59faabc9dedfc1f6bcb6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":159,"context_line":"            \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone):"},{"line_number":160,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":161,"context_line":"                self.ctxt, self.policy.id, self.rule_data)"},{"line_number":162,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_a67dd8e8","line":159,"range":{"start_line":159,"start_character":46,"end_line":159,"end_character":63},"updated":"2017-04-14 21:13:19.000000000","message":"Not needed","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"f85dd115331092dbdef1143b06607d896bdbe3cd","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":159,"context_line":"            \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone):"},{"line_number":160,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":161,"context_line":"                self.ctxt, self.policy.id, self.rule_data)"},{"line_number":162,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_7d5e3ae7","line":159,"range":{"start_line":159,"start_character":46,"end_line":159,"end_character":63},"in_reply_to":"7ffa3b31_a67dd8e8","updated":"2017-04-15 05:40:25.000000000","message":"I think on the contrary , I would like this to return None, to be clear that the function is intended to move forward.\nI know this is not necessary but provides a proper definition to the mock","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                return_value\u003d_policy) as mock_qos_get_obj:"},{"line_number":171,"context_line":"            self.qos_plugin.create_policy_minimum_bandwidth_rule("},{"line_number":172,"context_line":"                self.ctxt, _policy.id, self.rule_data)"},{"line_number":173,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":174,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":175,"context_line":"            mock_get_rules.assert_called_once_with(self.ctxt, _policy.id)"},{"line_number":176,"context_line":"            mock_qos_get_obj.assert_called_once_with(self.ctxt, id\u003d_policy.id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_d332e5a7","line":173,"updated":"2017-04-14 21:13:19.000000000","message":"I played a little bit with these unit tests and pdb. getattr(rule_object, \"get_rules\") is exactly the same as mock_get_rules:\n\n(Pdb) getattr(rule_object, \"get_rules\")\n\u003cMagicMock name\u003d\u0027get_rules\u0027 id\u003d\u0027139818185342352\u0027\u003e\n(Pdb) mock_get_rules\n\u003cMagicMock name\u003d\u0027get_rules\u0027 id\u003d\u0027139818185342352\u0027\u003e\n(Pdb) \n\nSo this assertion and the one in L175 do a very similar thing. Why do we need both? I think we should only keep L175","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                self.ctxt, _policy.id, self.rule_data)"},{"line_number":173,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":174,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":175,"context_line":"            mock_get_rules.assert_called_once_with(self.ctxt, _policy.id)"},{"line_number":176,"context_line":"            mock_qos_get_obj.assert_called_once_with(self.ctxt, id\u003d_policy.id)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def test_create_policy_rule_check_rule_max_more_than_min(self):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_b3275965","line":175,"updated":"2017-04-14 21:13:19.000000000","message":"See my comment to L173","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                return_value\u003d_policy) as mock_qos_get_obj:"},{"line_number":185,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":186,"context_line":"                self.ctxt, _policy.id, self.rule_data)"},{"line_number":187,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":188,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":189,"context_line":"            mock_get_rules.assert_called_once_with(self.ctxt, _policy.id)"},{"line_number":190,"context_line":"            mock_qos_get_obj.assert_called_once_with(self.ctxt, id\u003d_policy.id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_732431c7","line":187,"updated":"2017-04-14 21:13:19.000000000","message":"Please see comment to line 173","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":224,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    def test_update_policy_rule_check_rule_min_less_than_max(self):"},{"line_number":227,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":228,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":229,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_490f442e","line":226,"range":{"start_line":226,"start_character":9,"end_line":226,"end_character":60},"updated":"2017-04-14 21:13:19.000000000","message":"This name reflects what L238-248 do, but not L227-236. Maybe we should split this test case in two","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"f85dd115331092dbdef1143b06607d896bdbe3cd","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":224,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    def test_update_policy_rule_check_rule_min_less_than_max(self):"},{"line_number":227,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":228,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":229,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_1d6df697","line":226,"range":{"start_line":226,"start_character":9,"end_line":226,"end_character":60},"in_reply_to":"7ffa3b31_490f442e","updated":"2017-04-15 05:40:25.000000000","message":"The lines 227-238 create the base policy and update it , while the main test case ( 238-246 ) actually check what we are trying to do.\nI think we can keep this in the same TC","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            return_value\u003d_policy):"},{"line_number":233,"context_line":"            self.qos_plugin.update_policy_bandwidth_limit_rule("},{"line_number":234,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":235,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":236,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        rules \u003d [self.rule, self.min_rule]"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_4994e48b","line":235,"updated":"2017-04-14 21:13:19.000000000","message":"Consider naming the neutron.objects.qos.rule.get_rules mock and assert like in L175","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            self.qos_plugin.update_policy_minimum_bandwidth_rule("},{"line_number":245,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":246,"context_line":"                self.policy.id, self.rule_data)"},{"line_number":247,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":248,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"    def test_update_policy_rule_check_rule_bwlimit_less_than_minbw(self):"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_89e1cc01","line":247,"updated":"2017-04-14 21:13:19.000000000","message":"See comment to L235","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":250,"context_line":"    def test_update_policy_rule_check_rule_bwlimit_less_than_minbw(self):"},{"line_number":251,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":252,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":253,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":254,"context_line":"                        return_value\u003d[self.rule]), mock.patch("},{"line_number":255,"context_line":"            \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":256,"context_line":"            return_value\u003d_policy):"},{"line_number":257,"context_line":"            self.qos_plugin.update_policy_bandwidth_limit_rule("},{"line_number":258,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":259,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":260,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":261,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":262,"context_line":"        data[\u0027minimum_bandwidth_rule\u0027][\u0027min_kbps\u0027] \u003d 1000"},{"line_number":263,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_895e6ce4","line":260,"range":{"start_line":253,"start_character":0,"end_line":260,"end_character":57},"updated":"2017-04-14 21:13:19.000000000","message":"This is the same as L229-236. Why do it again here? Besides, what these lines do doesn\u0027t reflect the name of this test case","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"f85dd115331092dbdef1143b06607d896bdbe3cd","unresolved":false,"context_lines":[{"line_number":250,"context_line":"    def test_update_policy_rule_check_rule_bwlimit_less_than_minbw(self):"},{"line_number":251,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":252,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":253,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":254,"context_line":"                        return_value\u003d[self.rule]), mock.patch("},{"line_number":255,"context_line":"            \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":256,"context_line":"            return_value\u003d_policy):"},{"line_number":257,"context_line":"            self.qos_plugin.update_policy_bandwidth_limit_rule("},{"line_number":258,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":259,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":260,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":261,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":262,"context_line":"        data[\u0027minimum_bandwidth_rule\u0027][\u0027min_kbps\u0027] \u003d 1000"},{"line_number":263,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_3d7af223","line":260,"range":{"start_line":253,"start_character":0,"end_line":260,"end_character":57},"in_reply_to":"7ffa3b31_895e6ce4","updated":"2017-04-15 05:40:25.000000000","message":"I agree they are similar, but my motivation was to check recursive updates, one where the update is successful and the second where the update fails.\nThats why I have this code.","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5ad5fcf6b6bcd3d2286294cc727523f2ef82bad6","unresolved":false,"context_lines":[{"line_number":271,"context_line":"    def test_update_policy_rule_check_rule_minbw_gr_than_bwlimit(self):"},{"line_number":272,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":273,"context_line":"        setattr(_policy, \"rules\", [self.min_rule])"},{"line_number":274,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":275,"context_line":"                        return_value\u003d[self.min_rule]), mock.patch("},{"line_number":276,"context_line":"            \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":277,"context_line":"            return_value\u003d_policy):"},{"line_number":278,"context_line":"            self.qos_plugin.update_policy_minimum_bandwidth_rule("},{"line_number":279,"context_line":"                self.ctxt, self.min_rule.id, self.policy.id, self.rule_data)"},{"line_number":280,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":281,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":282,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":283,"context_line":"        data[\u0027bandwidth_limit_rule\u0027][\u0027max_kbps\u0027] \u003d 1"},{"line_number":284,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_099b7c72","line":281,"range":{"start_line":274,"start_character":0,"end_line":281,"end_character":57},"updated":"2017-04-14 21:13:19.000000000","message":"This is essentially the same as L240-248. Why do it again here? Besides, what these lines do doesn\u0027t reflect the name of this test case","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"f85dd115331092dbdef1143b06607d896bdbe3cd","unresolved":false,"context_lines":[{"line_number":271,"context_line":"    def test_update_policy_rule_check_rule_minbw_gr_than_bwlimit(self):"},{"line_number":272,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":273,"context_line":"        setattr(_policy, \"rules\", [self.min_rule])"},{"line_number":274,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":275,"context_line":"                        return_value\u003d[self.min_rule]), mock.patch("},{"line_number":276,"context_line":"            \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":277,"context_line":"            return_value\u003d_policy):"},{"line_number":278,"context_line":"            self.qos_plugin.update_policy_minimum_bandwidth_rule("},{"line_number":279,"context_line":"                self.ctxt, self.min_rule.id, self.policy.id, self.rule_data)"},{"line_number":280,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":281,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":282,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":283,"context_line":"        data[\u0027bandwidth_limit_rule\u0027][\u0027max_kbps\u0027] \u003d 1"},{"line_number":284,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"7ffa3b31_5d7dbe1a","line":281,"range":{"start_line":274,"start_character":0,"end_line":281,"end_character":57},"in_reply_to":"7ffa3b31_099b7c72","updated":"2017-04-15 05:40:25.000000000","message":"ditto","commit_id":"6c5d34e5a415abfd0447c29a5199ca75b43af0e6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"e59327858e3762503531585f35c0df71ebbdf5a9","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            return_value\u003d_policy):"},{"line_number":231,"context_line":"            self.qos_plugin.update_policy_bandwidth_limit_rule("},{"line_number":232,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":233,"context_line":"            mock_get_rules.assert_called_once_with(self.ctxt, _policy.id)"},{"line_number":234,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        rules \u003d [self.rule, self.min_rule]"}],"source_content_type":"text/x-python","patch_set":21,"id":"7ffa3b31_080b6f8e","line":233,"range":{"start_line":233,"start_character":12,"end_line":233,"end_character":26},"updated":"2017-04-15 22:12:24.000000000","message":"This name needs to be declared when creating the mock, L228","commit_id":"1337bb5fb1f26b6a5d0ee6e5397c63cb4d2b37de"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"e59327858e3762503531585f35c0df71ebbdf5a9","unresolved":false,"context_lines":[{"line_number":242,"context_line":"            self.qos_plugin.update_policy_minimum_bandwidth_rule("},{"line_number":243,"context_line":"                self.ctxt, self.min_rule.id,"},{"line_number":244,"context_line":"                self.policy.id, self.rule_data)"},{"line_number":245,"context_line":"            mock_get_rules.assert_called_once_with(self.ctxt, _policy.id)"},{"line_number":246,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def test_update_policy_rule_check_rule_bwlimit_less_than_minbw(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7ffa3b31_e8ff2365","line":245,"updated":"2017-04-15 22:12:24.000000000","message":"This name needs to be declared when creating the mock, L239","commit_id":"1337bb5fb1f26b6a5d0ee6e5397c63cb4d2b37de"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"3b1cba23d247fcee9c12117472b40310341ded50","unresolved":false,"context_lines":[{"line_number":238,"context_line":"        setattr(_policy, \"rules\", rules)"},{"line_number":239,"context_line":"        with mock.patch(\u0027neutron.objects.qos.rule.get_rules\u0027,"},{"line_number":240,"context_line":"                        return_value\u003drules) as mock_get_rules, \\"},{"line_number":241,"context_line":"             mock.patch("},{"line_number":242,"context_line":"                 \u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":243,"context_line":"                 return_value\u003d_policy):"},{"line_number":244,"context_line":"            self.qos_plugin.update_policy_minimum_bandwidth_rule("}],"source_content_type":"text/x-python","patch_set":22,"id":"5ff73747_6388f670","line":241,"range":{"start_line":241,"start_character":12,"end_line":241,"end_character":13},"updated":"2017-04-21 23:13:25.000000000","message":"Yeah, the line number in the pep8 message is messed up. It really refers to this line. You just need to remove one space here.\n\nI ran pep8 on your patchset locally and it worked fine, though:\n\npep8 runtests: commands[2] | flake8\n./neutron/tests/unit/services/qos/test_qos_plugin.py:241:14: E127 continuation line over-indented for visual indent\n             mock.patch(\n\nI haven\u0027t rebuilt my local tox pep8 environment in several weeks, so maybe something changed upstream","commit_id":"ed86efc04d62223a54f1e14fff5082ce7a81550e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"115ab22b2724dd7b6d55b8a1c9f4d6d593d31a9c","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":159,"context_line":"            \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone):"},{"line_number":160,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":161,"context_line":"                self.ctxt, self.policy.id, self.rule_data)"},{"line_number":162,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_1fc5fe8e","line":159,"updated":"2017-04-23 09:18:46.000000000","message":"nit: this \"return_value\u003dNone\" is probably not necessary here :)","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"f2cd597867d2ff107c85681bb1144e0e433fcaef","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":159,"context_line":"            \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone):"},{"line_number":160,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":161,"context_line":"                self.ctxt, self.policy.id, self.rule_data)"},{"line_number":162,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_a768dd20","line":159,"in_reply_to":"5ff73747_1fc5fe8e","updated":"2017-04-26 13:29:01.000000000","message":"Yeah, I know, but for clarity , I kept it ( this same comment was mentioned by others as well, I stated the same :) . Hope thats ok)","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2450d37a3a48c39750883816f3e20b4a5ffa401c","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":157,"context_line":"                        return_value\u003dself.policy), mock.patch("},{"line_number":158,"context_line":"            \u0027neutron.objects.qos.qos_policy_validator\u0027"},{"line_number":159,"context_line":"            \u0027.check_bandwidth_rule_conflict\u0027, return_value\u003dNone):"},{"line_number":160,"context_line":"            self.qos_plugin.create_policy_bandwidth_limit_rule("},{"line_number":161,"context_line":"                self.ctxt, self.policy.id, self.rule_data)"},{"line_number":162,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_30efb288","line":159,"in_reply_to":"5ff73747_a768dd20","updated":"2017-05-03 11:48:24.000000000","message":"Hmmmm, no. There is no need to return anything in this function, so this is no necesary. You should remove it.","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1ecb168f43ff2325a901584919c1dd6bee574b32","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_create_policy_rule_check_rule_bwlimit_less_than_minbw(self):"},{"line_number":191,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":192,"context_line":"        rule_data \u003d copy.deepcopy(self.rule_data)"},{"line_number":193,"context_line":"        rule_data[\u0027bandwidth_limit_rule\u0027][\u0027max_kbps\u0027] \u003d 1"},{"line_number":194,"context_line":"        setattr(_policy, \"rules\", [self.min_rule])"},{"line_number":195,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_55824751","line":192,"updated":"2017-04-23 19:41:27.000000000","message":"these deepcopys shouldn\u0027t be required. self.rule_data is constructed fresh on each test in setUp.","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2450d37a3a48c39750883816f3e20b4a5ffa401c","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_create_policy_rule_check_rule_bwlimit_less_than_minbw(self):"},{"line_number":191,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":192,"context_line":"        rule_data \u003d copy.deepcopy(self.rule_data)"},{"line_number":193,"context_line":"        rule_data[\u0027bandwidth_limit_rule\u0027][\u0027max_kbps\u0027] \u003d 1"},{"line_number":194,"context_line":"        setattr(_policy, \"rules\", [self.min_rule])"},{"line_number":195,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_9bc10784","line":192,"in_reply_to":"5ff73747_477a39f1","updated":"2017-05-03 11:48:24.000000000","message":"Kevin is right, there is no need for this.\n\nsetUp() function is executed everytime a test is executed (it\u0027s not a __init__ function)","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"f2cd597867d2ff107c85681bb1144e0e433fcaef","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_create_policy_rule_check_rule_bwlimit_less_than_minbw(self):"},{"line_number":191,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":192,"context_line":"        rule_data \u003d copy.deepcopy(self.rule_data)"},{"line_number":193,"context_line":"        rule_data[\u0027bandwidth_limit_rule\u0027][\u0027max_kbps\u0027] \u003d 1"},{"line_number":194,"context_line":"        setattr(_policy, \"rules\", [self.min_rule])"},{"line_number":195,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_477a39f1","line":192,"in_reply_to":"5ff73747_55824751","updated":"2017-04-26 13:29:01.000000000","message":"Ok, but the reason for the deepcopy is the modification in Line#193.\nBasically we are creating the original rule_data and modifying it in the Test case as per our test requirement.\nHope thats ok ?","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2450d37a3a48c39750883816f3e20b4a5ffa401c","unresolved":false,"context_lines":[{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def test_create_policy_rule_check_rule_minbw_gr_than_bwlimit(self):"},{"line_number":203,"context_line":"        _policy \u003d self._get_policy()"},{"line_number":204,"context_line":"        rule_data \u003d copy.deepcopy(self.rule_data)"},{"line_number":205,"context_line":"        rule_data[\u0027minimum_bandwidth_rule\u0027][\u0027min_kbps\u0027] \u003d 1000000"},{"line_number":206,"context_line":"        setattr(_policy, \"rules\", [self.rule])"},{"line_number":207,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_5bbb7f0d","line":204,"updated":"2017-05-03 11:48:24.000000000","message":"ditto","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2450d37a3a48c39750883816f3e20b4a5ffa401c","unresolved":false,"context_lines":[{"line_number":258,"context_line":"                self.ctxt, self.rule.id, self.policy.id, self.rule_data)"},{"line_number":259,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":260,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":261,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":262,"context_line":"        data[\u0027minimum_bandwidth_rule\u0027][\u0027min_kbps\u0027] \u003d 1000"},{"line_number":263,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":264,"context_line":"                        return_value\u003d_policy):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_bb496333","line":261,"updated":"2017-05-03 11:48:24.000000000","message":"ditto","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2450d37a3a48c39750883816f3e20b4a5ffa401c","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                self.ctxt, self.min_rule.id, self.policy.id, self.rule_data)"},{"line_number":280,"context_line":"            self.assertTrue(getattr(rule_object, \"get_rules\").called)"},{"line_number":281,"context_line":"            self._validate_driver_params(\u0027update_policy\u0027)"},{"line_number":282,"context_line":"        data \u003d copy.deepcopy(self.rule_data)"},{"line_number":283,"context_line":"        data[\u0027bandwidth_limit_rule\u0027][\u0027max_kbps\u0027] \u003d 1"},{"line_number":284,"context_line":"        with mock.patch(\u0027neutron.objects.qos.policy.QosPolicy.get_object\u0027,"},{"line_number":285,"context_line":"                        return_value\u003d_policy):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5ff73747_7b53dbe3","line":282,"updated":"2017-05-03 11:48:24.000000000","message":"ditto","commit_id":"ae795c53ed6b7a4ee571b385eaabdaed32f1da14"}]}