)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"f749edef9cc5e5ead89a4c5ef62f5f37f062d2e1","unresolved":false,"context_lines":[{"line_number":9,"context_line":"While inserting firewall rule into a policy"},{"line_number":10,"context_line":"firewall_rule should not be associated with"},{"line_number":11,"context_line":"any policy and ref_firewall_rule should be"},{"line_number":12,"context_line":"associated with the given policy only."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"},{"line_number":15,"context_line":"Closes-bug: 1330898"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1ae5cdf2_f18e9931","line":12,"updated":"2014-06-17 18:18:39.000000000","message":"This commit message is a bit confusing, can you rephrase it?","commit_id":"6ebc0903256ec425919e2abe0c2a18e41d509b6d"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"1695026708d2c416813395d6ab12a9db4c5224b5","unresolved":false,"context_lines":[{"line_number":9,"context_line":"While inserting firewall rule into a policy"},{"line_number":10,"context_line":"firewall_rule should not be associated with"},{"line_number":11,"context_line":"any policy and ref_firewall_rule should be"},{"line_number":12,"context_line":"associated with the given policy only."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"},{"line_number":15,"context_line":"Closes-bug: 1330898"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1ae5cdf2_7a928259","line":12,"in_reply_to":"1ae5cdf2_f18e9931","updated":"2014-06-24 10:56:40.000000000","message":"Done.","commit_id":"6ebc0903256ec425919e2abe0c2a18e41d509b6d"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"1a773f5936e10df74cf89cacd46f59d6fa131692","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Proper validation for inserting firewall rule"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While inserting rule r1 into a policy p1 before/after r2, r1 should not be"},{"line_number":10,"context_line":"associated with any policy. Rule r2 mentioned against insert_before/after"},{"line_number":11,"context_line":"should be associated with the given policy p1."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_0b94a9e6","line":10,"updated":"2014-06-25 10:30:13.000000000","message":"r1 not associated with any policy is the current behaviour - not something you\u0027re enforcing with this patch","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"73816088135441a2b700ffca8389ec8ed6e66abf","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Proper validation for inserting firewall rule"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While inserting rule r1 into a policy p1 before/after r2, r1 should not be"},{"line_number":10,"context_line":"associated with any policy. Rule r2 mentioned against insert_before/after"},{"line_number":11,"context_line":"should be associated with the given policy p1."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_c8e8de18","line":10,"in_reply_to":"1ae5cdf2_0b94a9e6","updated":"2014-06-26 06:24:56.000000000","message":"Yes, the validation for r1 is already there. Mentioned it for better understanding.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"1a773f5936e10df74cf89cacd46f59d6fa131692","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While inserting rule r1 into a policy p1 before/after r2, r1 should not be"},{"line_number":10,"context_line":"associated with any policy. Rule r2 mentioned against insert_before/after"},{"line_number":11,"context_line":"should be associated with the given policy p1."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"},{"line_number":14,"context_line":"Closes-bug: 1330898"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_aba9f526","line":11,"updated":"2014-06-25 10:30:13.000000000","message":"nit: consider...\n\"The rule r2 used a reference for insert position should be association with r1\u0027s policy, p1\"","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"f8a22215e170ce89973281aa9a68128cb56085ab","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While inserting rule r1 into a policy p1 before/after r2, r1 should not be"},{"line_number":10,"context_line":"associated with any policy. Rule r2 mentioned against insert_before/after"},{"line_number":11,"context_line":"should be associated with the given policy p1."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"},{"line_number":14,"context_line":"Closes-bug: 1330898"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_712dc194","line":11,"in_reply_to":"1ae5cdf2_686eea97","updated":"2014-07-04 22:59:03.000000000","message":"I don\u0027t think so. But since I\u0027m not a grammar teacher I won\u0027t insist.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"73816088135441a2b700ffca8389ec8ed6e66abf","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While inserting rule r1 into a policy p1 before/after r2, r1 should not be"},{"line_number":10,"context_line":"associated with any policy. Rule r2 mentioned against insert_before/after"},{"line_number":11,"context_line":"should be associated with the given policy p1."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"},{"line_number":14,"context_line":"Closes-bug: 1330898"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_686eea97","line":11,"in_reply_to":"1ae5cdf2_aba9f526","updated":"2014-06-26 06:24:56.000000000","message":"I think the current commit msg also gives same meaning and easy to understand.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"a5b046d6642eeffadea7d7661b65bd9c49666aa3","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While inserting rule r1 into a policy p1 before/after r2, r1 should not be"},{"line_number":10,"context_line":"associated with any policy. Rule r2 mentioned against insert_before/after"},{"line_number":11,"context_line":"should be associated with the given policy p1."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"},{"line_number":14,"context_line":"Closes-bug: 1330898"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_e87be377","line":11,"in_reply_to":"baada198_712dc194","updated":"2014-07-31 20:14:20.000000000","message":"I agree with Salvatore\u0027s suggestions here. Although I wrote some of the code that is being changed here (meaning I had enough context), I found it difficult to understand the commit message.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"c59b33dc00609781a7671dc74397ac9d96e44e57","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"While inserting rule r1 into a policy p1 before/after r2, r1 should not be"},{"line_number":10,"context_line":"associated with any policy. Rule r2 mentioned against insert_before/after"},{"line_number":11,"context_line":"should be associated with the given policy p1."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ifa415acc9533b7a323f966ee42d476460e68c9d3"},{"line_number":14,"context_line":"Closes-bug: 1330898"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_00003868","line":11,"in_reply_to":"baada198_e87be377","updated":"2014-08-06 13:17:31.000000000","message":"ok :). I have rewritten the commit message.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"}],"neutron/db/firewall/firewall_db.py":[{"author":{"_account_id":11825,"name":"Sudhakar Babu Gariganti","email":"sudhakar.gariganti@gmail.com","username":"sudhakar-gariganti"},"change_message_id":"b98b13688e4f6d91904195a4fadfe9d9b83da043","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":452,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":453,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":454,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":455,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":456,"context_line":"                        firewall_policy_id\u003did)"},{"line_number":457,"context_line":"                if insert_before:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_1f120e58","line":454,"updated":"2014-07-18 06:49:42.000000000","message":"Agree with Salvatore Orlando here.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"1a773f5936e10df74cf89cacd46f59d6fa131692","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":452,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":453,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":454,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":455,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":456,"context_line":"                        firewall_policy_id\u003did)"},{"line_number":457,"context_line":"                if insert_before:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_cb2d61aa","line":454,"updated":"2014-06-25 10:30:13.000000000","message":"perhaps a different exception should be considered for this case... something like ReferenceFirewallRulePolicyMismatch","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"f8a22215e170ce89973281aa9a68128cb56085ab","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":452,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":453,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":454,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":455,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":456,"context_line":"                        firewall_policy_id\u003did)"},{"line_number":457,"context_line":"                if insert_before:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_3118a974","line":454,"in_reply_to":"1ae5cdf2_88a89679","updated":"2014-07-04 22:59:03.000000000","message":"To me firewall rule not associated with policy means the rule has no policy.\nThe fact that it has already been used in another method does not mean anything to me. I\u0027d change it there too.\n\nBut as it seem you disagree on the fact the name is misleading, I won\u0027t insist.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"73816088135441a2b700ffca8389ec8ed6e66abf","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":452,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":453,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":454,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":455,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":456,"context_line":"                        firewall_policy_id\u003did)"},{"line_number":457,"context_line":"                if insert_before:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_88a89679","line":454,"in_reply_to":"1ae5cdf2_cb2d61aa","updated":"2014-06-26 06:24:56.000000000","message":"We are already using FirewallRuleNotAssociatedWithPolicy in remove firewall rule method. This exception suits the current situation also and no need of defining new exception.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"bf281ef176ea7f0551a47028417b2a3c808e4e59","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":452,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":453,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":454,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":455,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":456,"context_line":"                        firewall_policy_id\u003did)"},{"line_number":457,"context_line":"                if insert_before:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_7c8cf1d7","line":454,"in_reply_to":"baada198_1f120e58","updated":"2014-07-18 10:07:34.000000000","message":"The exception msg is \"Firewall Rule \u003crule_id\u003e is not associated with Firewall Policy \u003cpolicy_id\u003e\". So it clearly says rule to be inserted is not associated with the given policy. As it is displaying ids also, user will understand easily. IMO It will not convey the wrong meaning like \"rule has no policy\". If you people are not convinced I will change the msg. :)","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11825,"name":"Sudhakar Babu Gariganti","email":"sudhakar.gariganti@gmail.com","username":"sudhakar-gariganti"},"change_message_id":"a048afcfdc8ef38d02fafeb245b5fdf6ed16416b","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":452,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":453,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":454,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":455,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":456,"context_line":"                        firewall_policy_id\u003did)"},{"line_number":457,"context_line":"                if insert_before:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_f61c6715","line":454,"in_reply_to":"baada198_52f631d8","updated":"2014-07-21 06:15:21.000000000","message":"I think the conflict point here is about the variable which is taken as reference. This method has policy_id and reference_rule as parameters. You message is making sense when taken that policy id is correct, but an invalid rule is specified. I was thinking from the angle where the rule is correct but policy id is wrong. Perhaps Salvatore also gave comments in the same lines.\n\nI am changing to +1 now.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11825,"name":"Sudhakar Babu Gariganti","email":"sudhakar.gariganti@gmail.com","username":"sudhakar-gariganti"},"change_message_id":"332558e7f772dbb313d742d4e8d3f6edda55b1bd","unresolved":false,"context_lines":[{"line_number":451,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":452,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":453,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":454,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":455,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":456,"context_line":"                        firewall_policy_id\u003did)"},{"line_number":457,"context_line":"                if insert_before:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_52f631d8","line":454,"in_reply_to":"baada198_7c8cf1d7","updated":"2014-07-18 11:53:35.000000000","message":"In case of remove rule, the exception is being raised to prevent removing rules which are not belonging to the current policy. \nSo the context is slightly different there I feel.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11825,"name":"Sudhakar Babu Gariganti","email":"sudhakar.gariganti@gmail.com","username":"sudhakar-gariganti"},"change_message_id":"332558e7f772dbb313d742d4e8d3f6edda55b1bd","unresolved":false,"context_lines":[{"line_number":476,"context_line":"            raise firewall.FirewallRuleNotFound(firewall_rule_id\u003dNone)"},{"line_number":477,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":478,"context_line":"            fwr_db \u003d self._get_firewall_rule(context, firewall_rule_id)"},{"line_number":479,"context_line":"            if fwr_db.firewall_policy_id !\u003d id:"},{"line_number":480,"context_line":"                raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":481,"context_line":"                    firewall_rule_id\u003dfwr_db[\u0027id\u0027],"},{"line_number":482,"context_line":"                    firewall_policy_id\u003did)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_72edf5ef","line":479,"updated":"2014-07-18 11:53:35.000000000","message":"with the check added in insert method, doesn\u0027t this check become redundant in the remove method?","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"73039346e681fbce2448aad24ac224c1de56f717","unresolved":false,"context_lines":[{"line_number":476,"context_line":"            raise firewall.FirewallRuleNotFound(firewall_rule_id\u003dNone)"},{"line_number":477,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":478,"context_line":"            fwr_db \u003d self._get_firewall_rule(context, firewall_rule_id)"},{"line_number":479,"context_line":"            if fwr_db.firewall_policy_id !\u003d id:"},{"line_number":480,"context_line":"                raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":481,"context_line":"                    firewall_rule_id\u003dfwr_db[\u0027id\u0027],"},{"line_number":482,"context_line":"                    firewall_policy_id\u003did)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_9695eb33","line":479,"in_reply_to":"baada198_72edf5ef","updated":"2014-07-21 06:26:47.000000000","message":"That\u0027s true in case of GUI as it lists valid rules. But in case of CLI remove command, user can specify any rule which is not part of the policy.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11825,"name":"Sudhakar Babu Gariganti","email":"sudhakar.gariganti@gmail.com","username":"sudhakar-gariganti"},"change_message_id":"dc270a6f321abb781f5c5727e1f404acef2faafa","unresolved":false,"context_lines":[{"line_number":476,"context_line":"            raise firewall.FirewallRuleNotFound(firewall_rule_id\u003dNone)"},{"line_number":477,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":478,"context_line":"            fwr_db \u003d self._get_firewall_rule(context, firewall_rule_id)"},{"line_number":479,"context_line":"            if fwr_db.firewall_policy_id !\u003d id:"},{"line_number":480,"context_line":"                raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":481,"context_line":"                    firewall_rule_id\u003dfwr_db[\u0027id\u0027],"},{"line_number":482,"context_line":"                    firewall_policy_id\u003did)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_b6e82fb4","line":479,"in_reply_to":"baada198_9695eb33","updated":"2014-07-21 06:30:06.000000000","message":"Makes sense.\nThanks for clarifying.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"457a1ae5aab3892b42502bf24fe7c8edc4826cfb","unresolved":false,"context_lines":[{"line_number":452,"context_line":"                # rule is inserted after reference_firewall_rule_id."},{"line_number":453,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":454,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":455,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":456,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":457,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":458,"context_line":"                        firewall_policy_id\u003did)"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa98f980_dd07acc8","line":455,"updated":"2014-09-03 06:45:50.000000000","message":"I think we need an automated check for this if we want to meaningfully stop the shadowing of id. There are 500+ instances of it in the codebase.","commit_id":"c86b1f7723e4a4f768773a68fc3ac197ac751482"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"5266b143392042d05c9afcb1b06f15d8a09b3e46","unresolved":false,"context_lines":[{"line_number":452,"context_line":"                # rule is inserted after reference_firewall_rule_id."},{"line_number":453,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":454,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":455,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":456,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":457,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":458,"context_line":"                        firewall_policy_id\u003did)"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_c6dd75df","line":455,"updated":"2014-08-06 15:32:10.000000000","message":"Nit: id is a python function. Consider whether or not you\u0027d like to revise it for this function as part of the patch, or if you want to handle under a separate cleanup bug.","commit_id":"c86b1f7723e4a4f768773a68fc3ac197ac751482"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4bc82564a4248f3a653d2ad2e3eaacbc917e01b2","unresolved":false,"context_lines":[{"line_number":452,"context_line":"                # rule is inserted after reference_firewall_rule_id."},{"line_number":453,"context_line":"                ref_fwr_db \u003d self._get_firewall_rule("},{"line_number":454,"context_line":"                    context, ref_firewall_rule_id)"},{"line_number":455,"context_line":"                if ref_fwr_db.firewall_policy_id !\u003d id:"},{"line_number":456,"context_line":"                    raise firewall.FirewallRuleNotAssociatedWithPolicy("},{"line_number":457,"context_line":"                        firewall_rule_id\u003dref_fwr_db[\u0027id\u0027],"},{"line_number":458,"context_line":"                        firewall_policy_id\u003did)"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_58d2e4cf","line":455,"in_reply_to":"baada198_c6dd75df","updated":"2014-08-08 05:33:38.000000000","message":"I\u0027d say that is out of the scope of this patch so don\u0027t fix it here.","commit_id":"c86b1f7723e4a4f768773a68fc3ac197ac751482"}],"neutron/tests/unit/db/firewall/test_db_firewall.py":[{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"1a773f5936e10df74cf89cacd46f59d6fa131692","unresolved":false,"context_lines":[{"line_number":922,"context_line":"                    ref_fwr_id \u003d ref_fwr[\u0027firewall_rule\u0027][\u0027id\u0027]"},{"line_number":923,"context_line":"                    insert_data \u003d {\u0027firewall_rule_id\u0027: ref_fwr_id,"},{"line_number":924,"context_line":"                                   \u0027insert_before\u0027: ref_fwr_id,"},{"line_number":925,"context_line":"                                   \u0027insert_after\u0027: ref_fwr_id}"},{"line_number":926,"context_line":"                    self._rule_action("},{"line_number":927,"context_line":"                        \u0027insert\u0027, fwp_id, ref_fwr_id,"},{"line_number":928,"context_line":"                        expected_code\u003dwebob.exc.HTTPBadRequest.code,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_062580ad","line":925,"updated":"2014-06-25 10:30:13.000000000","message":"I think you should specify either insert before or insert after?\nI\u0027m not entirely sure whether this test is verifying that the rule referenced for insert position should be different from the one being inserted or that the rule being inserted and the one used as a reference must have the same policy....","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"73816088135441a2b700ffca8389ec8ed6e66abf","unresolved":false,"context_lines":[{"line_number":922,"context_line":"                    ref_fwr_id \u003d ref_fwr[\u0027firewall_rule\u0027][\u0027id\u0027]"},{"line_number":923,"context_line":"                    insert_data \u003d {\u0027firewall_rule_id\u0027: ref_fwr_id,"},{"line_number":924,"context_line":"                                   \u0027insert_before\u0027: ref_fwr_id,"},{"line_number":925,"context_line":"                                   \u0027insert_after\u0027: ref_fwr_id}"},{"line_number":926,"context_line":"                    self._rule_action("},{"line_number":927,"context_line":"                        \u0027insert\u0027, fwp_id, ref_fwr_id,"},{"line_number":928,"context_line":"                        expected_code\u003dwebob.exc.HTTPBadRequest.code,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_e3513d0f","line":925,"in_reply_to":"1ae5cdf2_062580ad","updated":"2014-06-26 06:24:56.000000000","message":"In the current code, if both are mentioned, insert_before is considered and insert_after will be ignored. This unit test is verifying that the reference rule should be associated with the policy and it is not verifying that Insert rule and reference rule should be same or different.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"f8a22215e170ce89973281aa9a68128cb56085ab","unresolved":false,"context_lines":[{"line_number":922,"context_line":"                    ref_fwr_id \u003d ref_fwr[\u0027firewall_rule\u0027][\u0027id\u0027]"},{"line_number":923,"context_line":"                    insert_data \u003d {\u0027firewall_rule_id\u0027: ref_fwr_id,"},{"line_number":924,"context_line":"                                   \u0027insert_before\u0027: ref_fwr_id,"},{"line_number":925,"context_line":"                                   \u0027insert_after\u0027: ref_fwr_id}"},{"line_number":926,"context_line":"                    self._rule_action("},{"line_number":927,"context_line":"                        \u0027insert\u0027, fwp_id, ref_fwr_id,"},{"line_number":928,"context_line":"                        expected_code\u003dwebob.exc.HTTPBadRequest.code,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_911ed55d","line":925,"in_reply_to":"1ae5cdf2_e3513d0f","updated":"2014-07-04 22:59:03.000000000","message":"In my opinion your test is verifying that the operation of inserting a rule is returning a 400. I can\u0027t find a proof that this is happening because ref_fwr_id is associated with a policy different from the one of the rule you\u0027re inserting.\n\nAnyway, I\u0027m pretty sure I\u0027m wrong here, so don\u0027t bother.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":11614,"name":"Koteswara Rao Kelam","email":"koteswara.kelam@hp.com","username":"koteswar"},"change_message_id":"c59b33dc00609781a7671dc74397ac9d96e44e57","unresolved":false,"context_lines":[{"line_number":922,"context_line":"                    ref_fwr_id \u003d ref_fwr[\u0027firewall_rule\u0027][\u0027id\u0027]"},{"line_number":923,"context_line":"                    insert_data \u003d {\u0027firewall_rule_id\u0027: ref_fwr_id,"},{"line_number":924,"context_line":"                                   \u0027insert_before\u0027: ref_fwr_id,"},{"line_number":925,"context_line":"                                   \u0027insert_after\u0027: ref_fwr_id}"},{"line_number":926,"context_line":"                    self._rule_action("},{"line_number":927,"context_line":"                        \u0027insert\u0027, fwp_id, ref_fwr_id,"},{"line_number":928,"context_line":"                        expected_code\u003dwebob.exc.HTTPBadRequest.code,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_c0a8007b","line":925,"in_reply_to":"baada198_68d273fe","updated":"2014-08-06 13:17:31.000000000","message":"thanks sumit. I modified and submitted new patch.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"a5b046d6642eeffadea7d7661b65bd9c49666aa3","unresolved":false,"context_lines":[{"line_number":922,"context_line":"                    ref_fwr_id \u003d ref_fwr[\u0027firewall_rule\u0027][\u0027id\u0027]"},{"line_number":923,"context_line":"                    insert_data \u003d {\u0027firewall_rule_id\u0027: ref_fwr_id,"},{"line_number":924,"context_line":"                                   \u0027insert_before\u0027: ref_fwr_id,"},{"line_number":925,"context_line":"                                   \u0027insert_after\u0027: ref_fwr_id}"},{"line_number":926,"context_line":"                    self._rule_action("},{"line_number":927,"context_line":"                        \u0027insert\u0027, fwp_id, ref_fwr_id,"},{"line_number":928,"context_line":"                        expected_code\u003dwebob.exc.HTTPBadRequest.code,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_68d273fe","line":925,"in_reply_to":"baada198_911ed55d","updated":"2014-07-31 20:14:20.000000000","message":"I think I agree with Salvatore here as well (and also based on the author\u0027s response). Currently only the case that the reference rule specificied in the \u0027insert_before\u0027 position does not belong to a firewall policy is getting tested. I think you need to a similar check for insert_after as well.","commit_id":"0d63327f1a3c3c6f5cebaf1a9a172803c05fb05b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4bc82564a4248f3a653d2ad2e3eaacbc917e01b2","unresolved":false,"context_lines":[{"line_number":939,"context_line":"                    self.firewall_policy(name\u003d\u0027fwp1\u0027,"},{"line_number":940,"context_line":"                                         firewall_rules\u003d[fwr1_id])) as fwp:"},{"line_number":941,"context_line":"                fwp0_id \u003d fwp[0][\u0027firewall_policy\u0027][\u0027id\u0027]"},{"line_number":942,"context_line":"                #test inserting before a rule which is associated"},{"line_number":943,"context_line":"                #with different policy"},{"line_number":944,"context_line":"                self._rule_action("},{"line_number":945,"context_line":"                    \u0027insert\u0027, fwp0_id, fwr0_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_f8e498a9","line":942,"updated":"2014-08-08 05:33:38.000000000","message":"nit:  I think it is customary to add a space after the #.","commit_id":"c86b1f7723e4a4f768773a68fc3ac197ac751482"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"69ed97ee3849e1aeff73f62c544b91143abb0849","unresolved":false,"context_lines":[{"line_number":939,"context_line":"                    self.firewall_policy(name\u003d\u0027fwp1\u0027,"},{"line_number":940,"context_line":"                                         firewall_rules\u003d[fwr1_id])) as fwp:"},{"line_number":941,"context_line":"                fwp0_id \u003d fwp[0][\u0027firewall_policy\u0027][\u0027id\u0027]"},{"line_number":942,"context_line":"                #test inserting before a rule which is associated"},{"line_number":943,"context_line":"                #with different policy"},{"line_number":944,"context_line":"                self._rule_action("},{"line_number":945,"context_line":"                    \u0027insert\u0027, fwp0_id, fwr0_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_2b9f78b4","line":942,"in_reply_to":"baada198_f8e498a9","updated":"2014-08-08 22:57:38.000000000","message":"I have seen it in so many places that I would encourage perhaps a HACKING check?","commit_id":"c86b1f7723e4a4f768773a68fc3ac197ac751482"}]}