)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"6d0aefe0bcc6e40585ecaff28f40a0b17514afc6","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adds comments to some of the iptables rules generated"},{"line_number":10,"context_line":"by neutron to assist with debugging."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1265493"},{"line_number":13,"context_line":"Change-Id: I7d9e37b9a43589dd7b142869b86fa15cb3fb329c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"AAAAW3%2F%2FOn8%3D","line":12,"updated":"2014-04-15 22:41:42.000000000","message":"perhaps you want to say that this partially fixes this bug (since I believe you are not adding comment to all the iptables rules).","commit_id":"8c1fb48eba97f591f0dcada80dbd0f30b618504a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"6a9faea4a184f7e56fbf859b0ee0ca7bb0dc23a6","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adds comments to some of the iptables rules generated"},{"line_number":10,"context_line":"by neutron to assist with debugging."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1265493"},{"line_number":13,"context_line":"Change-Id: I7d9e37b9a43589dd7b142869b86fa15cb3fb329c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"AAAAW3%2F%2FOnc%3D","line":12,"in_reply_to":"AAAAW3%2F%2FOn8%3D","updated":"2014-04-15 23:21:59.000000000","message":"Good call, there are still several rules that need comments.","commit_id":"8c1fb48eba97f591f0dcada80dbd0f30b618504a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1fc61c2cd574e3579b01f8beafdf9a56cc5bfa11","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adds comments to some of the iptables rules generated"},{"line_number":10,"context_line":"by neutron to assist with debugging."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Partial-Bug: #1265493"},{"line_number":13,"context_line":"Change-Id: I7d9e37b9a43589dd7b142869b86fa15cb3fb329c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"baa201ad_74bb75e5","line":12,"updated":"2014-10-01 17:09:13.000000000","message":"DocImpact, please","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"33326d3a80130d631985e38765347524eb43b421","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adds comments to some of the iptables rules generated"},{"line_number":10,"context_line":"by neutron to assist with debugging."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Partial-Bug: #1265493"},{"line_number":13,"context_line":"Change-Id: I7d9e37b9a43589dd7b142869b86fa15cb3fb329c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"baa201ad_53dc9d12","line":12,"in_reply_to":"baa201ad_74bb75e5","updated":"2014-10-03 07:31:13.000000000","message":"Done","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"}],"etc/neutron.conf":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"539a4fa85bb74b045fc740c6b7c43172ed595daf","unresolved":false,"context_lines":[{"line_number":545,"context_line":"# Change to \"sudo\" to skip the filtering and just run the comand directly"},{"line_number":546,"context_line":"# root_helper \u003d sudo"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""}],"source_content_type":"text/plain","patch_set":17,"id":"baa201ad_145a2950","line":548,"updated":"2014-10-01 17:25:32.000000000","message":"I looked at: \n\nhttps://review.openstack.org/#/c/94306\n\nWould there ever be a situation where an admin does not want the comments? Would it be a good idea if we determined whether the module is loaded at startup and we just use comments or not based on the availability of the module?\n\nIn other words, is it strictly necessary to have this config variable if we made this smart enough to determine whether to go w/ or w/o comments on its own?","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"33326d3a80130d631985e38765347524eb43b421","unresolved":false,"context_lines":[{"line_number":545,"context_line":"# Change to \"sudo\" to skip the filtering and just run the comand directly"},{"line_number":546,"context_line":"# root_helper \u003d sudo"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""}],"source_content_type":"text/plain","patch_set":17,"id":"baa201ad_d3e88db6","line":548,"in_reply_to":"baa201ad_145a2950","updated":"2014-10-03 07:31:13.000000000","message":":-(\n\nI prefer auto-detection of stuff and that\u0027s exactly what 94306 was doing but I was told that this needs to be configurable. You\u0027ll have to ask Mark why because I don\u0027t remember the reason at this point.","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"da5bddd8363455f9f22078b57bd341c327d46fb5","unresolved":false,"context_lines":[{"line_number":545,"context_line":"# Change to \"sudo\" to skip the filtering and just run the comand directly"},{"line_number":546,"context_line":"# root_helper \u003d sudo"},{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""}],"source_content_type":"text/plain","patch_set":17,"id":"baa201ad_2dcda366","line":548,"in_reply_to":"baa201ad_d3e88db6","updated":"2014-10-07 21:53:02.000000000","message":"+1  I don\u0027t remember the reason either.  But, okay.","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1fc61c2cd574e3579b01f8beafdf9a56cc5bfa11","unresolved":false,"context_lines":[{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"# \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d items for agent management extension \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":553,"context_line":"# seconds between nodes reporting state to server; should be less than"}],"source_content_type":"text/plain","patch_set":17,"id":"baa201ad_54ed91ee","line":550,"updated":"2014-10-01 17:09:13.000000000","message":"is it okay to have this ON by default? What\u0027s the failure mode if the module is not loaded?","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"da5bddd8363455f9f22078b57bd341c327d46fb5","unresolved":false,"context_lines":[{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"# \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d items for agent management extension \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":553,"context_line":"# seconds between nodes reporting state to server; should be less than"}],"source_content_type":"text/plain","patch_set":17,"id":"baa201ad_edd0fbca","line":550,"in_reply_to":"baa201ad_13d21500","updated":"2014-10-07 21:53:02.000000000","message":":P  I don\u0027t really want to bring up this discussion again.","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"33326d3a80130d631985e38765347524eb43b421","unresolved":false,"context_lines":[{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"# \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d items for agent management extension \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":553,"context_line":"# seconds between nodes reporting state to server; should be less than"}],"source_content_type":"text/plain","patch_set":17,"id":"baa201ad_13d21500","line":550,"in_reply_to":"baa201ad_54ed91ee","updated":"2014-10-03 07:31:13.000000000","message":"\u003eis it okay to have this ON by default?\n\nI think so. We couldn\u0027t find a modern distro without this module loaded.\n\n\u003eWhat\u0027s the failure mode if the module is not loaded?\n\nException diarrhea in the agent log.","commit_id":"da7978df9e120f036f60fe82a5856e6b2b19a4f5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"1d92cf04ad89012cb60c9f1f0d2922361853d623","unresolved":false,"context_lines":[{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"# \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d items for agent management extension \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":553,"context_line":"# seconds between nodes reporting state to server; should be less than"}],"source_content_type":"text/plain","patch_set":18,"id":"baa201ad_d1cb25a3","line":550,"updated":"2014-10-03 09:17:26.000000000","message":"Sorry, didn\u0027t quite get why this should be True by default. If a distro supports this it doesn\u0027t mean everybody needs it","commit_id":"ea202faec972d55b169f1c596209b4129aa64caf"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6910b9055d4adc3aa73db40a5adef9ae127a4793","unresolved":false,"context_lines":[{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"# \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d items for agent management extension \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":553,"context_line":"# seconds between nodes reporting state to server; should be less than"}],"source_content_type":"text/plain","patch_set":18,"id":"baa201ad_75c94925","line":550,"in_reply_to":"baa201ad_16083127","updated":"2014-10-07 06:44:50.000000000","message":"It\u0027s ok if it doesn\u0027t affect performance, can you confirm?","commit_id":"ea202faec972d55b169f1c596209b4129aa64caf"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"15817dd6cd5334fcc10bbdfe8f2bff9573996131","unresolved":false,"context_lines":[{"line_number":547,"context_line":""},{"line_number":548,"context_line":"# Set to true to add comments to generated iptables rules that describe"},{"line_number":549,"context_line":"# each rule\u0027s purpose. (System must support the iptables comments module.)"},{"line_number":550,"context_line":"# comment_iptables_rules \u003d True"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"# \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d items for agent management extension \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":553,"context_line":"# seconds between nodes reporting state to server; should be less than"}],"source_content_type":"text/plain","patch_set":18,"id":"baa201ad_16083127","line":550,"in_reply_to":"baa201ad_d1cb25a3","updated":"2014-10-03 21:11:41.000000000","message":"Why shouldn\u0027t it?","commit_id":"ea202faec972d55b169f1c596209b4129aa64caf"}],"neutron/agent/common/config.py":[{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"3e697bd6691638bf2d630b81b20196564e816fd0","unresolved":false,"context_lines":[{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"def register_iptables_opts(conf):"},{"line_number":103,"context_line":"    conf.register_opts(IPTABLES_OPTS, \u0027AGENT\u0027)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def get_root_helper(conf):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7adec928_0b581b31","line":103,"updated":"2014-05-23 19:33:28.000000000","message":"I\u0027m wondering if we should do a check to verify if the module is supported here.  While I understand the user would have to set this to True, if the system supported comments, wouldn\u0027t all users want it?\n\nI think the code could check to see if the comment module was loaded using the \u0027iptables -m comment\u0027 command.\n\nThis may reduce configuration options required by users, unless there are reasons where they wouldn\u0027t want this option even if the comments were supported by the system.","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ba856d079d688fc39f16b15d77254573de71f7aa","unresolved":false,"context_lines":[{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"def register_iptables_opts(conf):"},{"line_number":103,"context_line":"    conf.register_opts(IPTABLES_OPTS, \u0027AGENT\u0027)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def get_root_helper(conf):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7adec928_aeed75b9","line":103,"in_reply_to":"7adec928_0b581b31","updated":"2014-05-23 20:26:52.000000000","message":"So I have that approach as an alternative patch here:\n\nhttps://review.openstack.org/#/c/94306/","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"aa3d5db4f403cbe231eaee2a3b340f22a425dc6f","unresolved":false,"context_lines":[{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"def register_iptables_opts(conf):"},{"line_number":103,"context_line":"    conf.register_opts(IPTABLES_OPTS, \u0027AGENT\u0027)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def get_root_helper(conf):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7adec928_f192004a","line":103,"in_reply_to":"7adec928_aeed75b9","updated":"2014-05-23 20:58:12.000000000","message":"Is it either this patch, or the other patch...or could both go.  I briefly looked at the other and it seemed like it would supersede this one?","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"efb1e41dcdece8b9debfc43eb5015e7a24af1415","unresolved":false,"context_lines":[{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"def register_iptables_opts(conf):"},{"line_number":103,"context_line":"    conf.register_opts(IPTABLES_OPTS, \u0027AGENT\u0027)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def get_root_helper(conf):"}],"source_content_type":"text/x-python","patch_set":11,"id":"7adec928_9c18f376","line":103,"in_reply_to":"7adec928_f192004a","updated":"2014-05-23 21:13:40.000000000","message":"Yeah, it\u0027s one or the other right now.","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"}],"neutron/agent/linux/iptables_comments.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"008cf571c5a88a3189379eea5ac639f2498f9d57","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"\"\"\"iptables comments\"\"\""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"# Do not translate these comments without ensuring that nothing will have"},{"line_number":18,"context_line":"# a quote or escape character because they will end up in a call to iptables."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"SNAT_OUT \u003d \u0027Perform source NAT on outgoing traffic.\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"7ac7a964_d30c3d45","line":17,"updated":"2014-08-18 16:46:41.000000000","message":"Do not translate (period)\n\nIt might be clearer if you made your quotes and escape characters warning outside of the context of translation.","commit_id":"ae6d62f83120858c23a3725dbaf42fb494ef5215"}],"neutron/agent/linux/iptables_firewall.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5e32d27cc279139caa4e73b46e6778f899e1c075","unresolved":false,"context_lines":[{"line_number":171,"context_line":"                     \u0027-j $%s\u0027 % (self.IPTABLES_DIRECTION[direction],"},{"line_number":172,"context_line":"                                 device,"},{"line_number":173,"context_line":"                                 chain_name)]"},{"line_number":174,"context_line":"        comment \u003d _(\"Jump to the chain for device \u0027%s\u0027.\") % device"},{"line_number":175,"context_line":"        self._add_rule_to_chain_v4v6(SG_CHAIN, jump_rule, jump_rule,"},{"line_number":176,"context_line":"                                     comment\u003dcomment)"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWn%2F%2F2%2F4%3D","line":174,"updated":"2014-04-08 22:19:00.000000000","message":"I\u0027m not sure of the value of adding the specific device to the comment.  The device is clearly shown in the rule.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"0c0166c2002e605d1fbc78ee0d115d16ac19fd02","unresolved":false,"context_lines":[{"line_number":171,"context_line":"                     \u0027-j $%s\u0027 % (self.IPTABLES_DIRECTION[direction],"},{"line_number":172,"context_line":"                                 device,"},{"line_number":173,"context_line":"                                 chain_name)]"},{"line_number":174,"context_line":"        comment \u003d _(\"Jump to the chain for device \u0027%s\u0027.\") % device"},{"line_number":175,"context_line":"        self._add_rule_to_chain_v4v6(SG_CHAIN, jump_rule, jump_rule,"},{"line_number":176,"context_line":"                                     comment\u003dcomment)"},{"line_number":177,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F9qE%3D","line":174,"in_reply_to":"AAAAWn%2F%2F2%2F4%3D","updated":"2014-04-09 09:40:19.000000000","message":"Done","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5e32d27cc279139caa4e73b46e6778f899e1c075","unresolved":false,"context_lines":[{"line_number":284,"context_line":"            ipv4_sg_rules)"},{"line_number":285,"context_line":"        ipv6_iptables_rule +\u003d self._convert_sgr_to_iptables_rules("},{"line_number":286,"context_line":"            ipv6_sg_rules)"},{"line_number":287,"context_line":"        comment \u003d _(\"Rule from security group.\")"},{"line_number":288,"context_line":"        self._add_rule_to_chain_v4v6(chain_name,"},{"line_number":289,"context_line":"                                     ipv4_iptables_rule,"},{"line_number":290,"context_line":"                                     ipv6_iptables_rule)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWn%2F%2F3C0%3D","line":287,"updated":"2014-04-08 22:19:00.000000000","message":"This could be a little more descriptive.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"372b8b5668c55639b18efc69e6d14b4b4c4e437c","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        # Always drop invalid packets"},{"line_number":328,"context_line":"        iptables_rules +\u003d [\u0027-m state --state \u0027 \u0027INVALID -j DROP -m comment \u0027"},{"line_number":329,"context_line":"                           \u0027--comment \"Drop packets that are not associated \u0027"},{"line_number":330,"context_line":"                           \u0027with a state.\"\u0027]"},{"line_number":331,"context_line":"        return iptables_rules"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    def _allow_established(self, iptables_rules):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAW3%2F%2Ft6c%3D","line":330,"updated":"2014-04-10 21:57:56.000000000","message":"Example of a comment still not being translated.","commit_id":"0ee45fe58bf9f31b72fdb4b5a278b4304c9d3da5"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"256a3b5aafa6fa10c7681ca26ee5d8778e7123c8","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        ipv4_rules +\u003d [comment_rule(\u0027-p udp -m udp --sport 68 --dport 67 \u0027"},{"line_number":225,"context_line":"                                    \u0027-j RETURN\u0027, comment_key\u003d\u0027dhcp_client\u0027)]"},{"line_number":226,"context_line":"        ipv6_rules +\u003d [comment_rule(\u0027-p icmpv6 -j RETURN\u0027,"},{"line_number":227,"context_line":"                                    comment_key\u003d\u0027v6RAallow\u0027)]"},{"line_number":228,"context_line":"        ipv6_rules +\u003d [comment_rule(\u0027-p udp -m udp --sport 546 --dport 547 \u0027"},{"line_number":229,"context_line":"                                    \u0027-j RETURN\u0027, comment_key\u003dNone)]"},{"line_number":230,"context_line":"        mac_ipv4_pairs \u003d []"}],"source_content_type":"text/x-python","patch_set":9,"id":"7adec928_6b017b37","line":227,"updated":"2014-05-16 09:05:18.000000000","message":"nit:\n\nWhy not following the same style of the other comments.\n\nFor example: v6_RA_allow, or ipv6_RA_allow\n\nGood stuff BTW!","commit_id":"8228c988ef4ec104b8af78dc5b3cd8ac2946c4d9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"10d9a07249361ae28925753f94b6d98b03522f73","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        ipv4_rules +\u003d [comment_rule(\u0027-p udp -m udp --sport 68 --dport 67 \u0027"},{"line_number":225,"context_line":"                                    \u0027-j RETURN\u0027, comment_key\u003d\u0027dhcp_client\u0027)]"},{"line_number":226,"context_line":"        ipv6_rules +\u003d [comment_rule(\u0027-p icmpv6 -j RETURN\u0027,"},{"line_number":227,"context_line":"                                    comment_key\u003d\u0027v6RAallow\u0027)]"},{"line_number":228,"context_line":"        ipv6_rules +\u003d [comment_rule(\u0027-p udp -m udp --sport 546 --dport 547 \u0027"},{"line_number":229,"context_line":"                                    \u0027-j RETURN\u0027, comment_key\u003dNone)]"},{"line_number":230,"context_line":"        mac_ipv4_pairs \u003d []"}],"source_content_type":"text/x-python","patch_set":9,"id":"7adec928_27e2342f","line":227,"in_reply_to":"7adec928_6b017b37","updated":"2014-05-18 02:23:18.000000000","message":"Done","commit_id":"8228c988ef4ec104b8af78dc5b3cd8ac2946c4d9"},{"author":{"_account_id":6502,"name":"Andrey Epifanov","email":"aepifanov@mirantis.com","username":"aepifanov"},"change_message_id":"875d767c78dbaf8e9d32c43025ef6e016d574b18","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        ipv6_rules +\u003d [comment_rule(\u0027-p icmpv6 -j RETURN\u0027,"},{"line_number":255,"context_line":"                                    comment\u003dic.IPV6_RA_ALLOW)]"},{"line_number":256,"context_line":"        ipv6_rules +\u003d [comment_rule(\u0027-p udp -m udp --sport 546 --dport 547 \u0027"},{"line_number":257,"context_line":"                                    \u0027-j RETURN\u0027, comment\u003dNone)]"},{"line_number":258,"context_line":"        mac_ipv4_pairs \u003d []"},{"line_number":259,"context_line":"        mac_ipv6_pairs \u003d []"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"baa201ad_2bc68468","line":257,"updated":"2014-10-07 08:19:29.000000000","message":"Why this rule doesn\u0027t have a comment?","commit_id":"ea202faec972d55b169f1c596209b4129aa64caf"},{"author":{"_account_id":6502,"name":"Andrey Epifanov","email":"aepifanov@mirantis.com","username":"aepifanov"},"change_message_id":"875d767c78dbaf8e9d32c43025ef6e016d574b18","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        ipv4_rules +\u003d [comment_rule(\u0027-p udp -m udp --sport 67 --dport 68 \u0027"},{"line_number":283,"context_line":"                                    \u0027-j DROP\u0027, comment\u003dic.DHCP_SPOOF)]"},{"line_number":284,"context_line":"        ipv6_rules +\u003d [comment_rule(\u0027-p udp -m udp --sport 547 --dport 546 \u0027"},{"line_number":285,"context_line":"                                    \u0027-j DROP\u0027, comment\u003dNone)]"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    def _accept_inbound_icmpv6(self):"},{"line_number":288,"context_line":"        # Allow multicast listener, neighbor solicitation and"}],"source_content_type":"text/x-python","patch_set":18,"id":"baa201ad_aba99491","line":285,"updated":"2014-10-07 08:19:29.000000000","message":"The same Q.","commit_id":"ea202faec972d55b169f1c596209b4129aa64caf"}],"neutron/agent/linux/iptables_manager.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5e32d27cc279139caa4e73b46e6778f899e1c075","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.top \u003d top"},{"line_number":72,"context_line":"        self.wrap_name \u003d binary_name[:16]"},{"line_number":73,"context_line":"        self.tag \u003d tag"},{"line_number":74,"context_line":"        self.comment \u003d comment[0:255].replace(\u0027\"\u0027, \"\u0027\") if comment else None"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def __eq__(self, other):"},{"line_number":77,"context_line":"        return ((self.chain \u003d\u003d other.chain) and"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWn%2F%2F3D4%3D","line":74,"updated":"2014-04-08 22:19:00.000000000","message":"If you\u0027re going to check for \" then just throw an exception.  All of these strings are baked in to the source so they should follow the right format.\n\nI think I would avoid using % expansion and just hard-code all of the strings (e.g. don\u0027t embed a device name in the string as I said in another comment).  Then, illegal characters like \" can\u0027t sneak in and you don\u0027t have to validate anything here.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bcb0765fc4dd6e0f71e9ce6cd3226209f5015859","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.top \u003d top"},{"line_number":72,"context_line":"        self.wrap_name \u003d binary_name[:16]"},{"line_number":73,"context_line":"        self.tag \u003d tag"},{"line_number":74,"context_line":"        self.comment \u003d comment[0:255].replace(\u0027\"\u0027, \"\u0027\") if comment else None"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def __eq__(self, other):"},{"line_number":77,"context_line":"        return ((self.chain \u003d\u003d other.chain) and"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F5%2BM%3D","line":74,"in_reply_to":"AAAAW3%2F%2F9l4%3D","updated":"2014-04-09 15:28:49.000000000","message":"Unknown characters from translations is a whole different subject.  \" won\u0027t be one of them.  ;)","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"0c0166c2002e605d1fbc78ee0d115d16ac19fd02","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.top \u003d top"},{"line_number":72,"context_line":"        self.wrap_name \u003d binary_name[:16]"},{"line_number":73,"context_line":"        self.tag \u003d tag"},{"line_number":74,"context_line":"        self.comment \u003d comment[0:255].replace(\u0027\"\u0027, \"\u0027\") if comment else None"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def __eq__(self, other):"},{"line_number":77,"context_line":"        return ((self.chain \u003d\u003d other.chain) and"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F9l4%3D","line":74,"in_reply_to":"AAAAWn%2F%2F3D4%3D","updated":"2014-04-09 09:40:19.000000000","message":"I hardcoded the strings, but because of unknown characters from translations, I think I will still need to regex it out.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5e32d27cc279139caa4e73b46e6778f899e1c075","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            chain \u003d \u0027%s-%s\u0027 % (self.wrap_name, self.chain)"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            chain \u003d self.chain"},{"line_number":90,"context_line":"        if self.comment:"},{"line_number":91,"context_line":"            comment \u003d \u0027-m comment --comment \"%s\"\u0027 % self.comment"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            comment \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWn%2F%2F3C8%3D","line":90,"updated":"2014-04-08 22:19:00.000000000","message":"This should probably be unit tested at the least.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"0c0166c2002e605d1fbc78ee0d115d16ac19fd02","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            chain \u003d \u0027%s-%s\u0027 % (self.wrap_name, self.chain)"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            chain \u003d self.chain"},{"line_number":90,"context_line":"        if self.comment:"},{"line_number":91,"context_line":"            comment \u003d \u0027-m comment --comment \"%s\"\u0027 % self.comment"},{"line_number":92,"context_line":"        else:"},{"line_number":93,"context_line":"            comment \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F9lU%3D","line":90,"in_reply_to":"AAAAWn%2F%2F3C8%3D","updated":"2014-04-09 09:40:19.000000000","message":"It\u0027s definitely covered in the regular tests. ;-)","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5e32d27cc279139caa4e73b46e6778f899e1c075","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        return s"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def remove_rule(self, chain, rule, wrap\u003dTrue, top\u003dFalse, tag\u003dNone,"},{"line_number":217,"context_line":"                    comment\u003dNone):"},{"line_number":218,"context_line":"        \"\"\"Remove a rule from a chain."},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAWn%2F%2F3E0%3D","line":216,"updated":"2014-04-08 22:19:00.000000000","message":"This patch shouldn\u0027t add a tag argument just to pass it through.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2040321ac2679f0b8cace75fb4f4c97bdcf636e9","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        return s"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def remove_rule(self, chain, rule, wrap\u003dTrue, top\u003dFalse, tag\u003dNone,"},{"line_number":217,"context_line":"                    comment\u003dNone):"},{"line_number":218,"context_line":"        \"\"\"Remove a rule from a chain."},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F4Tw%3D","line":216,"in_reply_to":"AAAAW3%2F%2F59U%3D","updated":"2014-04-09 18:08:47.000000000","message":"The bug I thought was there was that the rules for matching on remove were never being constructed with the tag parameter so if tags were used they wouldn\u0027t match. \n\nHowever, after looking at it, the equality method for IptablesRule ignores tags anyway, so it doesn\u0027t matter.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bcb0765fc4dd6e0f71e9ce6cd3226209f5015859","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        return s"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def remove_rule(self, chain, rule, wrap\u003dTrue, top\u003dFalse, tag\u003dNone,"},{"line_number":217,"context_line":"                    comment\u003dNone):"},{"line_number":218,"context_line":"        \"\"\"Remove a rule from a chain."},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F59U%3D","line":216,"in_reply_to":"AAAAW3%2F%2F9lM%3D","updated":"2014-04-09 15:28:49.000000000","message":"What bug?  My comment is that you\u0027re adding \"tag\u003dNone\" to the argument list of this call for no apparent reason.  You don\u0027t need to pass tag to IptablesRule below.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"0c0166c2002e605d1fbc78ee0d115d16ac19fd02","unresolved":false,"context_lines":[{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        return s"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def remove_rule(self, chain, rule, wrap\u003dTrue, top\u003dFalse, tag\u003dNone,"},{"line_number":217,"context_line":"                    comment\u003dNone):"},{"line_number":218,"context_line":"        \"\"\"Remove a rule from a chain."},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAW3%2F%2F9lM%3D","line":216,"in_reply_to":"AAAAWn%2F%2F3E0%3D","updated":"2014-04-09 09:40:19.000000000","message":"I think this is a bug I should file separately.","commit_id":"360b82a89f30dcf5b1cbbe39dd2b6a631882af9c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f99e49a3bf0f79f5987bc3b364cd849aa467372f","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Even though all of the rule comments are defined in code,"},{"line_number":77,"context_line":"            # we need to remove any chars that could interfere with the"},{"line_number":78,"context_line":"            # command line that may be a result of a translation."},{"line_number":79,"context_line":"            self.comment \u003d re.sub(r\u0027[^a-zA-Z0-9 .\\\u0027]\u0027, \u0027\u0027, comment[0:255])"},{"line_number":80,"context_line":"        else:"},{"line_number":81,"context_line":"            self.comment \u003d None"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F5ww%3D","line":79,"updated":"2014-04-09 15:46:47.000000000","message":"I\u0027m curious, what characters are you concerned about?  If the message is translated to a language that uses unicode characters then those translations will be completely discarded by this regex.  Do unicode characters in general not work with iptables-save/restore?","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cf6b8897287ab91d1da3654cd2589c0b792efa3d","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Even though all of the rule comments are defined in code,"},{"line_number":77,"context_line":"            # we need to remove any chars that could interfere with the"},{"line_number":78,"context_line":"            # command line that may be a result of a translation."},{"line_number":79,"context_line":"            self.comment \u003d re.sub(r\u0027[^a-zA-Z0-9 .\\\u0027]\u0027, \u0027\u0027, comment[0:255])"},{"line_number":80,"context_line":"        else:"},{"line_number":81,"context_line":"            self.comment \u003d None"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2Ft%2B8%3D","line":79,"in_reply_to":"AAAAW3%2F%2F5aY%3D","updated":"2014-04-10 21:36:00.000000000","message":"These are fed to iptables-restore on a file descriptor which is written in C.  Not bash.","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"0db14c4fe6be9af643ca83e8f3b744901a8b208b","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Even though all of the rule comments are defined in code,"},{"line_number":77,"context_line":"            # we need to remove any chars that could interfere with the"},{"line_number":78,"context_line":"            # command line that may be a result of a translation."},{"line_number":79,"context_line":"            self.comment \u003d re.sub(r\u0027[^a-zA-Z0-9 .\\\u0027]\u0027, \u0027\u0027, comment[0:255])"},{"line_number":80,"context_line":"        else:"},{"line_number":81,"context_line":"            self.comment \u003d None"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F5aY%3D","line":79,"in_reply_to":"AAAAW3%2F%2F5ww%3D","updated":"2014-04-09 16:16:34.000000000","message":"Anything that would be interpreted as escape characters by bash since this is just run (with root privileges) on the command-line. I want to avoid a scenario where a new translation for one of these comments means you can no longer have security groups on deployments with that language.","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f99e49a3bf0f79f5987bc3b364cd849aa467372f","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            if not wrap:"},{"line_number":240,"context_line":"                self.remove_rules.append(IptablesRule(chain, rule, wrap, top,"},{"line_number":241,"context_line":"                                                      self.wrap_name,"},{"line_number":242,"context_line":"                                                      comment\u003dcomment))"},{"line_number":243,"context_line":"        except ValueError:"},{"line_number":244,"context_line":"            LOG.warn(_(\u0027Tried to remove rule that was not there:\u0027"},{"line_number":245,"context_line":"                       \u0027 %(chain)r %(rule)r %(wrap)r %(top)r\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F5wY%3D","line":242,"updated":"2014-04-09 15:46:47.000000000","message":":)","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f99e49a3bf0f79f5987bc3b364cd849aa467372f","unresolved":false,"context_lines":[{"line_number":353,"context_line":"            self.ipv4[\u0027nat\u0027].add_chain(\u0027snat\u0027)"},{"line_number":354,"context_line":"            self.ipv4[\u0027nat\u0027].add_rule(\u0027neutron-postrouting-bottom\u0027,"},{"line_number":355,"context_line":"                                      \u0027-j $snat\u0027, wrap\u003dFalse,"},{"line_number":356,"context_line":"                                      comment\u003d\u0027Perform source NAT on \u0027"},{"line_number":357,"context_line":"                                              \u0027outgoing traffic.\u0027)"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"            # And then we add a float-snat chain and jump to first thing in"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F53E%3D","line":356,"updated":"2014-04-09 15:46:47.000000000","message":"Be consistent with translations.  This one is not translated.  Others are.","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"8035ce1359151a3ec07dc04075b94e7bcaac93ef","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    \u0027statelessdrop\u0027: _(\u0027Drop packets that are not associated with a state.\u0027),"},{"line_number":72,"context_line":"    \u0027allowassoc\u0027: _(\u0027Direct packets associated with a known session to the \u0027"},{"line_number":73,"context_line":"                    \u0027RETURN chain.\u0027)"},{"line_number":74,"context_line":"}.iteritems())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def get_chain_name(chain_name, wrap\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAW3%2F%2FP%2BY%3D","line":74,"updated":"2014-04-15 19:31:28.000000000","message":"My only nit here is that these keys are pretty cryptic, for example \"sgtovmsg\", even \"sg_to_vm_sg\" is better.  The more obvious the better.","commit_id":"8c1fb48eba97f591f0dcada80dbd0f30b618504a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3b86840392f6be4c0ec13b108638f298a89ae19","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    \u0027statelessdrop\u0027: _(\u0027Drop packets that are not associated with a state.\u0027),"},{"line_number":72,"context_line":"    \u0027allowassoc\u0027: _(\u0027Direct packets associated with a known session to the \u0027"},{"line_number":73,"context_line":"                    \u0027RETURN chain.\u0027)"},{"line_number":74,"context_line":"}.iteritems())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def get_chain_name(chain_name, wrap\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAXH%2F%2F%2F5E%3D","line":74,"in_reply_to":"AAAAW3%2F%2FOZs%3D","updated":"2014-04-16 00:20:53.000000000","message":"Maybe we need to add comments to the comment keys where they\u0027re used.\n\nI\u0027m kidding.  ;)","commit_id":"8c1fb48eba97f591f0dcada80dbd0f30b618504a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"6a9faea4a184f7e56fbf859b0ee0ca7bb0dc23a6","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    \u0027statelessdrop\u0027: _(\u0027Drop packets that are not associated with a state.\u0027),"},{"line_number":72,"context_line":"    \u0027allowassoc\u0027: _(\u0027Direct packets associated with a known session to the \u0027"},{"line_number":73,"context_line":"                    \u0027RETURN chain.\u0027)"},{"line_number":74,"context_line":"}.iteritems())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def get_chain_name(chain_name, wrap\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAW3%2F%2FOZs%3D","line":74,"in_reply_to":"AAAAW3%2F%2FOoI%3D","updated":"2014-04-15 23:21:59.000000000","message":"I did the underscore versions, and it only required a few adjustments.","commit_id":"8c1fb48eba97f591f0dcada80dbd0f30b618504a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"924804aa02789330c9a6ee79bee9ddf259b5f1ba","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    \u0027statelessdrop\u0027: _(\u0027Drop packets that are not associated with a state.\u0027),"},{"line_number":72,"context_line":"    \u0027allowassoc\u0027: _(\u0027Direct packets associated with a known session to the \u0027"},{"line_number":73,"context_line":"                    \u0027RETURN chain.\u0027)"},{"line_number":74,"context_line":"}.iteritems())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def get_chain_name(chain_name, wrap\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAW3%2F%2FP4Y%3D","line":74,"in_reply_to":"AAAAW3%2F%2FP%2BY%3D","updated":"2014-04-15 19:38:21.000000000","message":"This was something I went back and forth on when writing it. Even if it\u0027s \"sg_to_vm_sg\", the dev is going to have to look here to see what it translates to anyway. It seemed like the additional line-wrapping caused by longer key-names wasn\u0027t worth it. I can be swayed though. :-)\n\nDo you think the underscores are worth the extra length?","commit_id":"8c1fb48eba97f591f0dcada80dbd0f30b618504a"},{"author":{"_account_id":490,"name":"Sumit Naiksatam","email":"sumitnaiksatam@gmail.com","username":"snaiksat"},"change_message_id":"6d0aefe0bcc6e40585ecaff28f40a0b17514afc6","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    \u0027statelessdrop\u0027: _(\u0027Drop packets that are not associated with a state.\u0027),"},{"line_number":72,"context_line":"    \u0027allowassoc\u0027: _(\u0027Direct packets associated with a known session to the \u0027"},{"line_number":73,"context_line":"                    \u0027RETURN chain.\u0027)"},{"line_number":74,"context_line":"}.iteritems())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def get_chain_name(chain_name, wrap\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAW3%2F%2FOoI%3D","line":74,"in_reply_to":"AAAAW3%2F%2FP4Y%3D","updated":"2014-04-15 22:41:42.000000000","message":"The underscore makes is more readable, but most people dealing with this are going to know the context anyway, so I agree with leaning in the direction of brevity (and lesser wrap arounds).","commit_id":"8c1fb48eba97f591f0dcada80dbd0f30b618504a"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"5ad294b29ab936a02ab7f93d53a12dedc637e6b6","unresolved":false,"context_lines":[{"line_number":53,"context_line":"# and escapes back-slashes and double-quotes that may be the result"},{"line_number":54,"context_line":"# of some translation."},{"line_number":55,"context_line":"def cclean(comment):"},{"line_number":56,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":59,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAXH%2F%2F%2Fpc%3D","line":56,"updated":"2014-04-16 01:15:43.000000000","message":"What if the comment contains \\\" ? That will be converted to \\\\\\\", is that OK? Isn\u0027t is safer to replace unwanted characters with underscores? Being paranoid I would ensure comment contains only [-_a-zA-Z0-9] and space. But I may just be crazy.","commit_id":"df704c7b92eeb593a190c15006d9bb6b95a8210a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"57c8eda73f0eca009a5ecf0fbda126ec291baf6e","unresolved":false,"context_lines":[{"line_number":53,"context_line":"# and escapes back-slashes and double-quotes that may be the result"},{"line_number":54,"context_line":"# of some translation."},{"line_number":55,"context_line":"def cclean(comment):"},{"line_number":56,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":59,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAXH%2F%2F%2FH8%3D","line":56,"in_reply_to":"AAAAXH%2F%2F%2Fpc%3D","updated":"2014-04-16 03:01:29.000000000","message":"\\\" gets converted to \\\\\\\", which is OK. one escape stops the escape, the other escapes the quote as expected. Here is a sample iptables comment generated from -m comment --comment \"alskdfj\\\\\\\"\":\n\nufw-track-input  all  --  anywhere             anywhere             /* alskdfj\\\" */","commit_id":"df704c7b92eeb593a190c15006d9bb6b95a8210a"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"5ad294b29ab936a02ab7f93d53a12dedc637e6b6","unresolved":false,"context_lines":[{"line_number":70,"context_line":"    \u0027unmatched\u0027: _(\u0027Send unmatched traffic to the fallback chain.\u0027),"},{"line_number":71,"context_line":"    \u0027stateless_drop\u0027: _(\u0027Drop packets that are not associated with a state.\u0027),"},{"line_number":72,"context_line":"    \u0027allow_assoc\u0027: _(\u0027Direct packets associated with a known session to the \u0027"},{"line_number":73,"context_line":"                     \u0027RETURN chain.\u0027)"},{"line_number":74,"context_line":"}.iteritems())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAXH%2F%2F%2Fq0%3D","line":73,"updated":"2014-04-16 01:15:43.000000000","message":"Just for the record, my opinion is that we should NOT translate these strings. Translating means we are vulnerable to crashing because of some weird unicode character combination that is not mitigated by cclean. I could even imagine it being used as an attack vector if the translation process is insecure. But again, I am probably just crazy.","commit_id":"df704c7b92eeb593a190c15006d9bb6b95a8210a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"57c8eda73f0eca009a5ecf0fbda126ec291baf6e","unresolved":false,"context_lines":[{"line_number":70,"context_line":"    \u0027unmatched\u0027: _(\u0027Send unmatched traffic to the fallback chain.\u0027),"},{"line_number":71,"context_line":"    \u0027stateless_drop\u0027: _(\u0027Drop packets that are not associated with a state.\u0027),"},{"line_number":72,"context_line":"    \u0027allow_assoc\u0027: _(\u0027Direct packets associated with a known session to the \u0027"},{"line_number":73,"context_line":"                     \u0027RETURN chain.\u0027)"},{"line_number":74,"context_line":"}.iteritems())"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAXH%2F%2F%2FKo%3D","line":73,"in_reply_to":"AAAAXH%2F%2F%2Fq0%3D","updated":"2014-04-16 03:01:29.000000000","message":"This was my initial thought too, but I jammed all kinds of weird unicode chars into the comments section and piped it into iptables-restore successfully. Check out my comment on patch set 3.","commit_id":"df704c7b92eeb593a190c15006d9bb6b95a8210a"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"4e1b76a3185cbbd65a85f7292dc1881ed24d0f71","unresolved":false,"context_lines":[{"line_number":58,"context_line":"def cclean(comment):"},{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_b9e58c0a","line":61,"updated":"2014-05-18 10:40:40.000000000","message":"should the comments be translated?","commit_id":"d4f9f5772df0d15d4330046efd1d976ad99d4740"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e393ff3bd52c9157e4596a99b39ffc50fb931988","unresolved":false,"context_lines":[{"line_number":58,"context_line":"def cclean(comment):"},{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_0c5aa382","line":61,"in_reply_to":"7adec928_b9e58c0a","updated":"2014-05-19 05:17:15.000000000","message":"I think so. The iptables comments support unicode so we might as well utilize it to localize operational data.","commit_id":"d4f9f5772df0d15d4330046efd1d976ad99d4740"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2febe96687605af7ef0953583a1f927e575eef0e","unresolved":false,"context_lines":[{"line_number":78,"context_line":"}.iteritems())"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def comment_rule(rule, comment_key):"},{"line_number":82,"context_line":"    if not cfg.CONF.AGENT.comment_iptables_rules or not comment_key:"},{"line_number":83,"context_line":"        return rule"},{"line_number":84,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":11,"id":"7adec928_203c3bbf","line":81,"updated":"2014-05-22 16:20:40.000000000","message":"Maybe an obvious question, but why didn\u0027t you default comment_key to None and save the callers that don\u0027t care from passing it ?  If it\u0027s mainly to make sure someone that copies some code knows there\u0027s this argument that\u0027s fine by me.","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"259a463f9be062cbc6da817ec8ab7c36f482af95","unresolved":false,"context_lines":[{"line_number":78,"context_line":"}.iteritems())"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def comment_rule(rule, comment_key):"},{"line_number":82,"context_line":"    if not cfg.CONF.AGENT.comment_iptables_rules or not comment_key:"},{"line_number":83,"context_line":"        return rule"},{"line_number":84,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":11,"id":"7adec928_c2447776","line":81,"in_reply_to":"7adec928_203c3bbf","updated":"2014-05-22 19:56:31.000000000","message":"I don\u0027t think there is a case where someone would call comment_rule and not provide the key for the comment since it wouldn\u0027t do anything.","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"cb7239434a58a8ffa5f8df8e6ed76727919973f4","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    try:"},{"line_number":85,"context_line":"        return \u0027%s -m comment --comment \"%s\"\u0027 % (rule, COMMENTS[comment_key])"},{"line_number":86,"context_line":"    except KeyError:"},{"line_number":87,"context_line":"        raise Exception(_(\"No iptables comment with key \u0027%s\u0027\"), comment_key)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"def get_chain_name(chain_name, wrap\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5adbc538_34f118ce","line":87,"updated":"2014-05-26 09:00:57.000000000","message":"Why does this need to be a fatal error?  Seems like we could avoid setting the comment when the key is not available and LOG.debug()","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"4dd52e8a512fb4c53abf4683307050f4609a9278","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    try:"},{"line_number":85,"context_line":"        return \u0027%s -m comment --comment \"%s\"\u0027 % (rule, COMMENTS[comment_key])"},{"line_number":86,"context_line":"    except KeyError:"},{"line_number":87,"context_line":"        raise Exception(_(\"No iptables comment with key \u0027%s\u0027\"), comment_key)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"def get_chain_name(chain_name, wrap\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5adbc538_c70596c3","line":87,"in_reply_to":"5adbc538_34f118ce","updated":"2014-05-26 20:11:26.000000000","message":"Since these are set in the code, there really shouldn\u0027t be a case where the the key references a non-existent comment. I made it fatal so a developer would catch the mistake early on.","commit_id":"af5104c81f431989ba178b527288fa8a9c5a4422"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"82163a399e1cd48da0be3634406c53fe91a2afc6","unresolved":false,"context_lines":[{"line_number":58,"context_line":"def cclean(comment):"},{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"1ae5cdf2_4afd4fb0","line":61,"updated":"2014-06-05 17:48:59.000000000","message":"have we validated that the translated strings actually appear in this case?  I think the cleaning at module import time does not allow the context to properly evaluated to select the lang","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cfa55df937ce97d2a5141adcaf97dfa460de6860","unresolved":false,"context_lines":[{"line_number":58,"context_line":"def cclean(comment):"},{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"1ae5cdf2_4b03f50f","line":61,"in_reply_to":"1ae5cdf2_4afd4fb0","updated":"2014-06-11 03:58:12.000000000","message":"+1 to confirmation of Mark\u0027s question.  Alse, we no longer translate strings in debug log messages.  Granted, this is a slightly different scenario but is it close enough?  These are strings used for debugging.  Do they no longer need to be translated?","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"ef3f02e3a5879029bc1f2fe7d7e170b2977ee9c2","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"},{"line_number":65,"context_line":"                   \u0027group chain.\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"3ae8d1ca_149a4863","line":62,"updated":"2014-05-27 21:27:46.000000000","message":"I\u0027m concerned about usage of strings as keys. Would it better to define constants and use them instead?","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"82163a399e1cd48da0be3634406c53fe91a2afc6","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"},{"line_number":65,"context_line":"                   \u0027group chain.\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"1ae5cdf2_6a2f6b3f","line":62,"updated":"2014-06-05 17:48:59.000000000","message":"Well not really an extra layer of indirection.","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"e91f455ecc6babc54704e87965633cd458629d3a","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"},{"line_number":65,"context_line":"                   \u0027group chain.\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"1ae5cdf2_3b4773bd","line":62,"in_reply_to":"1ae5cdf2_3c33e9f6","updated":"2014-06-13 16:16:38.000000000","message":"Are we seriously having this discussion?","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"fea9e041bf266c96de671d864ccfb82c2584e001","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"},{"line_number":65,"context_line":"                   \u0027group chain.\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"1ae5cdf2_3c33e9f6","line":62,"in_reply_to":"1ae5cdf2_6a2f6b3f","updated":"2014-06-05 20:18:02.000000000","message":"Isn\u0027t this the definition of indirection? Using a variable referencing a string rather than using a literal string.","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"be6bcab6b9c6fee55d1344604661f480a810dd56","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"},{"line_number":65,"context_line":"                   \u0027group chain.\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"1ae5cdf2_e8967a3e","line":62,"in_reply_to":"1ae5cdf2_8145e3b4","updated":"2014-06-03 18:14:53.000000000","message":"In both cases, an error is immediately thrown on evaluation [1]. Since everything passes through the comment_rule function, I could just remove the catch for KeyErrors as well. The one advantage I can see is that flake8 will recognize when a global in the same module isn\u0027t defined, but it still doesn\u0027t recognize undefined references to other modules. Am I missing any other advantages?\n\nThe reason I\u0027m hesitant to switch to constants is that they will become annoyingly long when used in other modules. For example:\n\nself._add_rule_to_chain_v4v6(\u0027INPUT\u0027, jump_rule, jump_rule,\n                                            comment_key\u003diptables_manager.INPUT_TO_SG)\n\n\n1.\n\n\u003e\u003e\u003e mydict \u003d {\u0027f\u0027: 1}\n\u003e\u003e\u003e def test():\n...     print mydict[SOMECONST]\n...\n\u003e\u003e\u003e def test2():\n...     print mydict[\u0027g\u0027]\n...\n\u003e\u003e\u003e test()\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"\u003cstdin\u003e\", line 2, in test\nNameError: global name \u0027SOMECONST\u0027 is not defined\n\u003e\u003e\u003e test2()\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"\u003cstdin\u003e\", line 2, in test2\nKeyError: \u0027g\u0027\n\u003e\u003e\u003e","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"5676c292fed19adbf05c99b322a8aeb62e3d89e5","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"},{"line_number":65,"context_line":"                   \u0027group chain.\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"3ae8d1ca_b1ffdb00","line":62,"in_reply_to":"3ae8d1ca_149a4863","updated":"2014-05-28 03:52:13.000000000","message":"What are the advantages of the extra layer of indirection?","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"720ea92bc9ef638f28d52bc57b25d7250f50b518","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    return comment[0:255].replace(\u0027\\\\\u0027, \u0027\\\\\\\\\u0027).replace(\u0027\"\u0027, \u0027\\\"\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"COMMENTS \u003d dict((key, cclean(value)) for key, value in {"},{"line_number":62,"context_line":"    \u0027snat_out\u0027: _(\u0027Perform source NAT on outgoing traffic.\u0027),"},{"line_number":63,"context_line":"    \u0027unmatch_drop\u0027: _(\u0027Default drop rule for unmatched traffic.\u0027),"},{"line_number":64,"context_line":"    \u0027vm_int_sg\u0027: _(\u0027Direct traffic from the VM interface to the security \u0027"},{"line_number":65,"context_line":"                   \u0027group chain.\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"1ae5cdf2_8145e3b4","line":62,"in_reply_to":"3ae8d1ca_b1ffdb00","updated":"2014-06-03 10:28:56.000000000","message":"+1 for constant keys\n\nIt could be irrelevant on this exact example BTW:\n\nFor example, if you use the wrong string for the key, it would fail instantly at evaluation time, because the constant doesn\u0027t exist.\n\nI the other hand, if you use the wrong string key in a dict, the error could be hidden for a while...","commit_id":"a068750044179e8f79a5cb1e879de771106d8fbb"}],"neutron/tests/unit/test_iptables_firewall.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f99e49a3bf0f79f5987bc3b364cd849aa467372f","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        ingress \u003d call.add_rule("},{"line_number":249,"context_line":"            \u0027ifake_dev\u0027,"},{"line_number":250,"context_line":"            \u0027-s %s -p tcp -m tcp -m multiport --dports 10:100 \u0027"},{"line_number":251,"context_line":"            \u0027-j RETURN\u0027 % prefix, comment\u003dNone)"},{"line_number":252,"context_line":"        egress \u003d None"},{"line_number":253,"context_line":"        self._test_prepare_port_filter(rule, ingress, egress)"},{"line_number":254,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F5vE%3D","line":251,"updated":"2014-04-09 15:46:47.000000000","message":"None is the default, right?  There is no reason to add this to the tests.  Remove all of the places where the tests add \"comment\u003dNone\" when None is the default anyway.\n\nIt makes this patch huge and more difficult to merge with other patches that might touch this file.","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cf6b8897287ab91d1da3654cd2589c0b792efa3d","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        ingress \u003d call.add_rule("},{"line_number":249,"context_line":"            \u0027ifake_dev\u0027,"},{"line_number":250,"context_line":"            \u0027-s %s -p tcp -m tcp -m multiport --dports 10:100 \u0027"},{"line_number":251,"context_line":"            \u0027-j RETURN\u0027 % prefix, comment\u003dNone)"},{"line_number":252,"context_line":"        egress \u003d None"},{"line_number":253,"context_line":"        self._test_prepare_port_filter(rule, ingress, egress)"},{"line_number":254,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2Ft9Y%3D","line":251,"in_reply_to":"AAAAW3%2F%2F5eA%3D","updated":"2014-04-10 21:36:00.000000000","message":"Right, my mistake.  I did not fully understand where \"call\" was coming from.  :)","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"0db14c4fe6be9af643ca83e8f3b744901a8b208b","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        ingress \u003d call.add_rule("},{"line_number":249,"context_line":"            \u0027ifake_dev\u0027,"},{"line_number":250,"context_line":"            \u0027-s %s -p tcp -m tcp -m multiport --dports 10:100 \u0027"},{"line_number":251,"context_line":"            \u0027-j RETURN\u0027 % prefix, comment\u003dNone)"},{"line_number":252,"context_line":"        egress \u003d None"},{"line_number":253,"context_line":"        self._test_prepare_port_filter(rule, ingress, egress)"},{"line_number":254,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F5eA%3D","line":251,"in_reply_to":"AAAAW3%2F%2F5vE%3D","updated":"2014-04-09 16:16:34.000000000","message":"I can\u0027t here because add_rule is called with comment\u003dNone in these cases and the assertion will fail without them. \n\nThe other option is to change the logic that calls add_rule from _add_rule_to_chain_v4v6 (in iptables_firewall.py) so it doesn\u0027t specify \u0027comment\u003dcomment\u0027 if it is None.","commit_id":"cf45327555b0f78f3e3c26a0dca9780a99fc5db9"}]}