)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"8fd0baea79a7bd66a06707787d3cafdbae8cc290","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     IWAMOTO Toshihiro \u003ciwamoto@valinux.co.jp\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-08-14 10:07:25 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"ovsfw: Fix port_ranges handling for sctp and numeric protocols"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I6c810a152990246d42d98c3673c4b5ee126ebb4b"},{"line_number":10,"context_line":"Closes-bug: #1708580"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f436f4f_f3d3fa5e","line":7,"range":{"start_line":7,"start_character":50,"end_line":7,"end_character":62},"updated":"2017-08-14 21:55:12.000000000","message":"Header line is too long. It has to be at most 50 characters long.\n\nCan you also add a few lines giving explaining with a little more details what this patchset is?","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"2b29fd07638b793b82c99d36a3a63af469bec583","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     IWAMOTO Toshihiro \u003ciwamoto@valinux.co.jp\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-08-14 10:07:25 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"ovsfw: Fix port_ranges handling for sctp and numeric protocols"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I6c810a152990246d42d98c3673c4b5ee126ebb4b"},{"line_number":10,"context_line":"Closes-bug: #1708580"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f436f4f_a1119c5f","line":7,"range":{"start_line":7,"start_character":50,"end_line":7,"end_character":62},"in_reply_to":"9f436f4f_f3d3fa5e","updated":"2017-08-15 01:00:27.000000000","message":"Done","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"}],"neutron/agent/linux/openvswitch_firewall/firewall.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cf9eb7d08c68f13860df01e346e7e8beebb296a3","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Only map protocols that need special handling"},{"line_number":35,"context_line":"REVERSE_IP_PROTOCOL_MAP \u003d dict(["},{"line_number":36,"context_line":"    (lib_const.IP_PROTOCOL_MAP[proto], proto) for proto in"},{"line_number":37,"context_line":"    [lib_const.PROTO_NAME_ICMP,"},{"line_number":38,"context_line":"     lib_const.PROTO_NAME_IPV6_ICMP,"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_b94e686c","line":35,"range":{"start_line":35,"start_character":26,"end_line":35,"end_character":31},"updated":"2017-08-04 10:30:54.000000000","message":"nit: You can use dict comprehension directly instead of creating a list of tuples and then retype.","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"c1d1b25a0b886a4671071b68c61f980bc94b3d66","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Only map protocols that need special handling"},{"line_number":35,"context_line":"REVERSE_IP_PROTOCOL_MAP \u003d dict(["},{"line_number":36,"context_line":"    (lib_const.IP_PROTOCOL_MAP[proto], proto) for proto in"},{"line_number":37,"context_line":"    [lib_const.PROTO_NAME_ICMP,"},{"line_number":38,"context_line":"     lib_const.PROTO_NAME_IPV6_ICMP,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_b8bbb80c","line":35,"range":{"start_line":35,"start_character":26,"end_line":35,"end_character":31},"in_reply_to":"df3967d1_b94e686c","updated":"2017-08-08 08:33:41.000000000","message":"Done","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"3d8450cca6177329beb7c4dd3ca9de7407f58b7e","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Only map protocols that need special handling"},{"line_number":35,"context_line":"REVERSE_IP_PROTOCOL_MAP \u003d dict(["},{"line_number":36,"context_line":"    (lib_const.IP_PROTOCOL_MAP[proto], proto) for proto in"},{"line_number":37,"context_line":"    [lib_const.PROTO_NAME_ICMP,"},{"line_number":38,"context_line":"     lib_const.PROTO_NAME_IPV6_ICMP,"},{"line_number":39,"context_line":"     lib_const.PROTO_NAME_SCTP,"},{"line_number":40,"context_line":"     lib_const.PROTO_NAME_UDP,"},{"line_number":41,"context_line":"     lib_const.PROTO_NAME_TCP]])"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"def _replace_register(flow_params, register_number, register_value):"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_ad6986f9","line":41,"range":{"start_line":35,"start_character":0,"end_line":41,"end_character":32},"updated":"2017-08-04 08:23:46.000000000","message":"Why only these protocols? According to the documentation and, for example, the OSC client, many others are accepted (in str and int mode):\n\nOSC, v2/security_group_rule)\nL159\nhelp\u003d_(\"IP protocol (ah, dccp, egp, esp, gre, icmp, igmp, \"\n                   \"ipv6-encap, ipv6-frag, ipv6-icmp, ipv6-nonxt, \"\n                   \"ipv6-opts, ipv6-route, ospf, pgm, rsvp, sctp, tcp, \"\n                   \"udp, udplite, vrrp and integer representations [0-255]; \"\n                   \"default: tcp)\")\n\nL240\nif (protocol.startswith(\u0027ipv6-\u0027) or\n                protocol in [\u0027icmpv6\u0027, \u002741\u0027, \u002743\u0027, \u002744\u0027, \u002758\u0027, \u002759\u0027, \u002760\u0027]):\n\n\nIMO, in this file in L99 you should take a look at neutron_lib.IP_PROTOCOL_MAP.items() and obtain the key (string) for this value (int).","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"38d7c38d82326534425ed02d09247b2aad4d48df","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Only map protocols that need special handling"},{"line_number":35,"context_line":"REVERSE_IP_PROTOCOL_MAP \u003d dict(["},{"line_number":36,"context_line":"    (lib_const.IP_PROTOCOL_MAP[proto], proto) for proto in"},{"line_number":37,"context_line":"    [lib_const.PROTO_NAME_ICMP,"},{"line_number":38,"context_line":"     lib_const.PROTO_NAME_IPV6_ICMP,"},{"line_number":39,"context_line":"     lib_const.PROTO_NAME_SCTP,"},{"line_number":40,"context_line":"     lib_const.PROTO_NAME_UDP,"},{"line_number":41,"context_line":"     lib_const.PROTO_NAME_TCP]])"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"def _replace_register(flow_params, register_number, register_value):"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_b9df8767","line":41,"range":{"start_line":35,"start_character":0,"end_line":41,"end_character":32},"in_reply_to":"df3967d1_a890544b","updated":"2017-08-04 11:12:35.000000000","message":"As commented by Jakub in L101, could be better to have protocol always as a number.\n\nOnce you have the protocol as a number, you can handle icmp with \"create_icmp_flows\" and protocols with port in \"create_port_range_flows\".\n\nMy aim with this comment is to:\n- Avoid having special lists of protocols like this one. You already have a function for protocols with ports filtering by protocol and a function for icmp flows called when the protocol is ICMP (v4, v6)\n- Have a unique way to represent protocols, strings or numbers. But I think Jakub\u0027s proposal is better than mine.","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"614502c5d939fbfa8625e31d9a4f835d3175748b","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Only map protocols that need special handling"},{"line_number":35,"context_line":"REVERSE_IP_PROTOCOL_MAP \u003d dict(["},{"line_number":36,"context_line":"    (lib_const.IP_PROTOCOL_MAP[proto], proto) for proto in"},{"line_number":37,"context_line":"    [lib_const.PROTO_NAME_ICMP,"},{"line_number":38,"context_line":"     lib_const.PROTO_NAME_IPV6_ICMP,"},{"line_number":39,"context_line":"     lib_const.PROTO_NAME_SCTP,"},{"line_number":40,"context_line":"     lib_const.PROTO_NAME_UDP,"},{"line_number":41,"context_line":"     lib_const.PROTO_NAME_TCP]])"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"def _replace_register(flow_params, register_number, register_value):"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_a890544b","line":41,"range":{"start_line":35,"start_character":0,"end_line":41,"end_character":32},"in_reply_to":"df3967d1_ad6986f9","updated":"2017-08-04 08:40:20.000000000","message":"Please look at the comment just above.\n\nrule[\u0027protocol\u0027] is converted to a number to form a match field anyway.\nThis reverse mapping is for protocols that require additional handling (like port_range or icmp_type etc).","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"c1d1b25a0b886a4671071b68c61f980bc94b3d66","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Only map protocols that need special handling"},{"line_number":35,"context_line":"REVERSE_IP_PROTOCOL_MAP \u003d dict(["},{"line_number":36,"context_line":"    (lib_const.IP_PROTOCOL_MAP[proto], proto) for proto in"},{"line_number":37,"context_line":"    [lib_const.PROTO_NAME_ICMP,"},{"line_number":38,"context_line":"     lib_const.PROTO_NAME_IPV6_ICMP,"},{"line_number":39,"context_line":"     lib_const.PROTO_NAME_SCTP,"},{"line_number":40,"context_line":"     lib_const.PROTO_NAME_UDP,"},{"line_number":41,"context_line":"     lib_const.PROTO_NAME_TCP]])"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"def _replace_register(flow_params, register_number, register_value):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_58404c08","line":41,"range":{"start_line":35,"start_character":0,"end_line":41,"end_character":32},"in_reply_to":"df3967d1_b9df8767","updated":"2017-08-08 08:33:41.000000000","message":"Done","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cf9eb7d08c68f13860df01e346e7e8beebb296a3","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def update_rules(self, rules):"},{"line_number":95,"context_line":"        \"\"\"Separate raw and remote rules.\"\"\""},{"line_number":96,"context_line":"        for rule in rules:"},{"line_number":97,"context_line":"            protocol \u003d rule.get(\u0027protocol\u0027)"},{"line_number":98,"context_line":"            if protocol is not None and protocol.isdigit():"},{"line_number":99,"context_line":"                str_protocol \u003d REVERSE_IP_PROTOCOL_MAP.get(int(protocol))"},{"line_number":100,"context_line":"                if str_protocol is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_797b607a","line":97,"range":{"start_line":96,"start_character":26,"end_line":97,"end_character":8},"updated":"2017-08-04 10:30:54.000000000","message":"I think now then we have 3 times going over rules, we could do one loop and categorize raw_rules and remote_rules here too.","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"c1d1b25a0b886a4671071b68c61f980bc94b3d66","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def update_rules(self, rules):"},{"line_number":95,"context_line":"        \"\"\"Separate raw and remote rules.\"\"\""},{"line_number":96,"context_line":"        for rule in rules:"},{"line_number":97,"context_line":"            protocol \u003d rule.get(\u0027protocol\u0027)"},{"line_number":98,"context_line":"            if protocol is not None and protocol.isdigit():"},{"line_number":99,"context_line":"                str_protocol \u003d REVERSE_IP_PROTOCOL_MAP.get(int(protocol))"},{"line_number":100,"context_line":"                if str_protocol is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_b8c5f85d","line":97,"range":{"start_line":96,"start_character":26,"end_line":97,"end_character":8},"in_reply_to":"df3967d1_797b607a","updated":"2017-08-08 08:33:41.000000000","message":"Done","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cf9eb7d08c68f13860df01e346e7e8beebb296a3","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if protocol is not None and protocol.isdigit():"},{"line_number":99,"context_line":"                str_protocol \u003d REVERSE_IP_PROTOCOL_MAP.get(int(protocol))"},{"line_number":100,"context_line":"                if str_protocol is not None:"},{"line_number":101,"context_line":"                    rule[\u0027protocol\u0027] \u003d str_protocol"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        self.raw_rules \u003d [rule for rule in rules"},{"line_number":104,"context_line":"                          if \u0027remote_group_id\u0027 not in rule]"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_d98b6c65","line":101,"range":{"start_line":101,"start_character":20,"end_line":101,"end_character":51},"updated":"2017-08-04 10:30:54.000000000","message":"What would be the advantage of translating numbers to strings? Isn\u0027t it better vice-versa - to translate strings into int, so from now on we always have a number.","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"c1d1b25a0b886a4671071b68c61f980bc94b3d66","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if protocol is not None and protocol.isdigit():"},{"line_number":99,"context_line":"                str_protocol \u003d REVERSE_IP_PROTOCOL_MAP.get(int(protocol))"},{"line_number":100,"context_line":"                if str_protocol is not None:"},{"line_number":101,"context_line":"                    rule[\u0027protocol\u0027] \u003d str_protocol"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        self.raw_rules \u003d [rule for rule in rules"},{"line_number":104,"context_line":"                          if \u0027remote_group_id\u0027 not in rule]"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f436f4f_98a0f4a7","line":101,"range":{"start_line":101,"start_character":20,"end_line":101,"end_character":51},"in_reply_to":"df3967d1_d98b6c65","updated":"2017-08-08 08:33:41.000000000","message":"Done","commit_id":"ee5407a35f329ce5fffdf857d81fc8aae1097710"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"56ccb598aab57e7989aee7bc907855318cfb3b6f","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        \"\"\"Separate raw and remote rules.\"\"\""},{"line_number":87,"context_line":"        self.raw_rules \u003d []"},{"line_number":88,"context_line":"        self.remote_rules \u003d []"},{"line_number":89,"context_line":"        for rule in rules:"},{"line_number":90,"context_line":"            protocol \u003d rule.get(\u0027protocol\u0027)"},{"line_number":91,"context_line":"            if protocol is not None and not protocol.isdigit():"},{"line_number":92,"context_line":"                if (rule.get(\u0027ethertype\u0027) \u003d\u003d lib_const.IPv6 and"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f436f4f_ce90f698","line":89,"updated":"2017-08-08 11:03:52.000000000","message":"I agree with dealing only with protocol numbers and making here the transformation.\n\n+1","commit_id":"b721d02af73e02c5246c8701fb2e559125387e7e"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3ab1cfd10017b262f59dac0e43fcac15c39d649f","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        self.raw_rules \u003d []"},{"line_number":88,"context_line":"        self.remote_rules \u003d []"},{"line_number":89,"context_line":"        for rule in rules:"},{"line_number":90,"context_line":"            protocol \u003d rule.get(\u0027protocol\u0027)"},{"line_number":91,"context_line":"            if protocol is not None:"},{"line_number":92,"context_line":"                if protocol.isdigit():"},{"line_number":93,"context_line":"                    rule[\u0027protocol\u0027] \u003d int(protocol)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_bf4a3552","line":90,"range":{"start_line":90,"start_character":0,"end_line":90,"end_character":43},"updated":"2017-08-10 12:19:40.000000000","message":"Can you separate the logic for obtaining protocol number into a separate function and then just\n\n rule[\u0027protocol\u0027] \u003d get_protocol_number_from_rule(rule)\n\nYou can also then cover such method with unittest so we know it behaves correctly.","commit_id":"fe32be93c156269f5af84fd8cbe42d9fab093760"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fbe2ede57c344c67aa1426a71fbc0a91b8457b8f","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        self.raw_rules \u003d []"},{"line_number":88,"context_line":"        self.remote_rules \u003d []"},{"line_number":89,"context_line":"        for rule in rules:"},{"line_number":90,"context_line":"            protocol \u003d rule.get(\u0027protocol\u0027)"},{"line_number":91,"context_line":"            if protocol is not None:"},{"line_number":92,"context_line":"                if protocol.isdigit():"},{"line_number":93,"context_line":"                    rule[\u0027protocol\u0027] \u003d int(protocol)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_2bf007fe","line":90,"range":{"start_line":90,"start_character":0,"end_line":90,"end_character":43},"in_reply_to":"9f436f4f_bf4a3552","updated":"2017-08-14 01:07:42.000000000","message":"Added some unit tests.\n\nThis function is reasonable small so splitting this out into another function isn\u0027t needed, IMHO.","commit_id":"fe32be93c156269f5af84fd8cbe42d9fab093760"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3ab1cfd10017b262f59dac0e43fcac15c39d649f","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                    rule[\u0027protocol\u0027] \u003d int(protocol)"},{"line_number":94,"context_line":"                else:"},{"line_number":95,"context_line":"                    if (rule.get(\u0027ethertype\u0027) \u003d\u003d lib_const.IPv6 and"},{"line_number":96,"context_line":"                        protocol \u003d\u003d lib_const.PROTO_NAME_ICMP):"},{"line_number":97,"context_line":"                        rule[\u0027protocol\u0027] \u003d lib_const.PROTO_NUM_IPV6_ICMP"},{"line_number":98,"context_line":"                    else:"},{"line_number":99,"context_line":"                        rule[\u0027protocol\u0027] \u003d lib_const.IP_PROTOCOL_MAP.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_bf1c756a","line":96,"updated":"2017-08-10 12:19:40.000000000","message":"Consider indentation","commit_id":"fe32be93c156269f5af84fd8cbe42d9fab093760"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fbe2ede57c344c67aa1426a71fbc0a91b8457b8f","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                    rule[\u0027protocol\u0027] \u003d int(protocol)"},{"line_number":94,"context_line":"                else:"},{"line_number":95,"context_line":"                    if (rule.get(\u0027ethertype\u0027) \u003d\u003d lib_const.IPv6 and"},{"line_number":96,"context_line":"                        protocol \u003d\u003d lib_const.PROTO_NAME_ICMP):"},{"line_number":97,"context_line":"                        rule[\u0027protocol\u0027] \u003d lib_const.PROTO_NUM_IPV6_ICMP"},{"line_number":98,"context_line":"                    else:"},{"line_number":99,"context_line":"                        rule[\u0027protocol\u0027] \u003d lib_const.IP_PROTOCOL_MAP.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_0bf1c301","line":96,"in_reply_to":"9f436f4f_bf1c756a","updated":"2017-08-14 01:07:42.000000000","message":"Done","commit_id":"fe32be93c156269f5af84fd8cbe42d9fab093760"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"8fd0baea79a7bd66a06707787d3cafdbae8cc290","unresolved":false,"context_lines":[{"line_number":91,"context_line":"            if protocol is not None:"},{"line_number":92,"context_line":"                if protocol.isdigit():"},{"line_number":93,"context_line":"                    rule[\u0027protocol\u0027] \u003d int(protocol)"},{"line_number":94,"context_line":"                else:"},{"line_number":95,"context_line":"                    if (rule.get(\u0027ethertype\u0027) \u003d\u003d lib_const.IPv6 and"},{"line_number":96,"context_line":"                            protocol \u003d\u003d lib_const.PROTO_NAME_ICMP):"},{"line_number":97,"context_line":"                        rule[\u0027protocol\u0027] \u003d lib_const.PROTO_NUM_IPV6_ICMP"},{"line_number":98,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_b306921b","line":95,"range":{"start_line":94,"start_character":0,"end_line":95,"end_character":67},"updated":"2017-08-14 21:55:12.000000000","message":"These two lines can be merged in a elif .....","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"2b29fd07638b793b82c99d36a3a63af469bec583","unresolved":false,"context_lines":[{"line_number":91,"context_line":"            if protocol is not None:"},{"line_number":92,"context_line":"                if protocol.isdigit():"},{"line_number":93,"context_line":"                    rule[\u0027protocol\u0027] \u003d int(protocol)"},{"line_number":94,"context_line":"                else:"},{"line_number":95,"context_line":"                    if (rule.get(\u0027ethertype\u0027) \u003d\u003d lib_const.IPv6 and"},{"line_number":96,"context_line":"                            protocol \u003d\u003d lib_const.PROTO_NAME_ICMP):"},{"line_number":97,"context_line":"                        rule[\u0027protocol\u0027] \u003d lib_const.PROTO_NUM_IPV6_ICMP"},{"line_number":98,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_011ce8be","line":95,"range":{"start_line":94,"start_character":0,"end_line":95,"end_character":67},"in_reply_to":"9f436f4f_b306921b","updated":"2017-08-15 01:00:27.000000000","message":"Done","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"}],"neutron/agent/linux/openvswitch_firewall/rules.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3ab1cfd10017b262f59dac0e43fcac15c39d649f","unresolved":false,"context_lines":[{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"def create_port_range_flows(flow_template, rule):"},{"line_number":103,"context_line":"    protocol \u003d ovsfw_consts.REVERSE_IP_PROTOCOL_MAP_WITH_PORTS.get("},{"line_number":104,"context_line":"        rule.get(\u0027protocol\u0027))"},{"line_number":105,"context_line":"    if protocol is None:"},{"line_number":106,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_5f3a2998","line":103,"updated":"2017-08-10 12:19:40.000000000","message":"It would be good to cover with unittests cases where names are passed and we validate correct int values in the flows.","commit_id":"fe32be93c156269f5af84fd8cbe42d9fab093760"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"fbe2ede57c344c67aa1426a71fbc0a91b8457b8f","unresolved":false,"context_lines":[{"line_number":100,"context_line":""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"def create_port_range_flows(flow_template, rule):"},{"line_number":103,"context_line":"    protocol \u003d ovsfw_consts.REVERSE_IP_PROTOCOL_MAP_WITH_PORTS.get("},{"line_number":104,"context_line":"        rule.get(\u0027protocol\u0027))"},{"line_number":105,"context_line":"    if protocol is None:"},{"line_number":106,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_cbde1b6c","line":103,"in_reply_to":"9f436f4f_5f3a2998","updated":"2017-08-14 01:07:42.000000000","message":"test_update_rules_protocols and test_create_protocol_flow_xxx combined covers that.","commit_id":"fe32be93c156269f5af84fd8cbe42d9fab093760"}],"neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"943498e5bb60dba9392ff031d2764c380889bacd","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_ICMP,"},{"line_number":75,"context_line":"             \u0027ethertype\u0027: constants.IPv6},"},{"line_number":76,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_TCP},"},{"line_number":77,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: \u0027baz\u0027}]"},{"line_number":78,"context_line":"        self.sg.update_rules(rules)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        protos \u003d [rule[\u0027protocol\u0027] for rule in self.sg.raw_rules]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_9ede050f","line":77,"updated":"2017-08-14 11:30:10.000000000","message":"There are missing cases where protocol is not defined, where protocol is given by number and ICMPv6","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"4bc75ea10ac99dfdcb7a8f22a367ddbdd45f1786","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_ICMP,"},{"line_number":75,"context_line":"             \u0027ethertype\u0027: constants.IPv6},"},{"line_number":76,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_TCP},"},{"line_number":77,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: \u0027baz\u0027}]"},{"line_number":78,"context_line":"        self.sg.update_rules(rules)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        protos \u003d [rule[\u0027protocol\u0027] for rule in self.sg.raw_rules]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_9315813f","line":77,"in_reply_to":"9f436f4f_5f55e5f8","updated":"2017-08-17 11:28:48.000000000","message":"I\u0027m not saying we should throw in more methods, just that each test should aim for given functionality and since update_rules now does two things - normalizing protocol and splitting rules, each test method should cover all cases even if they overlap.","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ef40a87fd0c3351669d64d953ace9719e5bcd7bd","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_ICMP,"},{"line_number":75,"context_line":"             \u0027ethertype\u0027: constants.IPv6},"},{"line_number":76,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_TCP},"},{"line_number":77,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: \u0027baz\u0027}]"},{"line_number":78,"context_line":"        self.sg.update_rules(rules)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        protos \u003d [rule[\u0027protocol\u0027] for rule in self.sg.raw_rules]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_5f55e5f8","line":77,"in_reply_to":"9f436f4f_90b0e9de","updated":"2017-08-17 04:42:30.000000000","message":"As update_rules now does more job than before, those two test funcs is okay IMO.\nI\u0027ll update the name of test_update_rules to clarify things.","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"2b29fd07638b793b82c99d36a3a63af469bec583","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_ICMP,"},{"line_number":75,"context_line":"             \u0027ethertype\u0027: constants.IPv6},"},{"line_number":76,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_TCP},"},{"line_number":77,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: \u0027baz\u0027}]"},{"line_number":78,"context_line":"        self.sg.update_rules(rules)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        protos \u003d [rule[\u0027protocol\u0027] for rule in self.sg.raw_rules]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_c1cdc031","line":77,"in_reply_to":"9f436f4f_9ede050f","updated":"2017-08-15 01:00:27.000000000","message":"Cases without protocol is covered above.","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"e597d7c53ca4c495702f64aca27c402cead74d96","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_ICMP,"},{"line_number":75,"context_line":"             \u0027ethertype\u0027: constants.IPv6},"},{"line_number":76,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_TCP},"},{"line_number":77,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: \u0027baz\u0027}]"},{"line_number":78,"context_line":"        self.sg.update_rules(rules)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        protos \u003d [rule[\u0027protocol\u0027] for rule in self.sg.raw_rules]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_447502cc","line":77,"in_reply_to":"9f436f4f_9ede050f","updated":"2017-08-18 01:06:37.000000000","message":"Done","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ad84eefbf742667f6cb77b87fc56962518573685","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_ICMP,"},{"line_number":75,"context_line":"             \u0027ethertype\u0027: constants.IPv6},"},{"line_number":76,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: constants.PROTO_NAME_TCP},"},{"line_number":77,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027protocol\u0027: \u0027baz\u0027}]"},{"line_number":78,"context_line":"        self.sg.update_rules(rules)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        protos \u003d [rule[\u0027protocol\u0027] for rule in self.sg.raw_rules]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f436f4f_90b0e9de","line":77,"in_reply_to":"9f436f4f_c1cdc031","updated":"2017-08-16 18:37:11.000000000","message":"That test aims the differentiation of rules into raw and remote as that was the only purpose of update_rules(). That means, the method on L59 should be renamed.\n\nIf there is a bug any of the case I described in previous comment, this test must fail, which currently won\u0027t happen.","commit_id":"b394991bd26c6a3565dbd474cc24b6f612783232"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"9752177138140b180d4f13f830815fcd95e7db78","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        self.sg \u003d ovsfw.SecurityGroup(\u0027123\u0027)"},{"line_number":57,"context_line":"        self.sg.members \u003d {\u0027type\u0027: [1, 2, 3, 4]}"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_update_rules_no_protocols(self):"},{"line_number":60,"context_line":"        rules \u003d ["},{"line_number":61,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027rule\u0027: \u0027all\u0027}, {\u0027bar\u0027: \u0027foo\u0027},"},{"line_number":62,"context_line":"            {\u0027remote_group_id\u0027: \u0027123456\u0027, \u0027foo\u0027: \u0027bar\u0027}]"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f436f4f_53527914","line":59,"range":{"start_line":59,"start_character":8,"end_line":59,"end_character":38},"updated":"2017-08-17 11:33:40.000000000","message":"This doesn\u0027t aim to validate what happens when you don\u0027t pass protocol but that rules are divided into raw and remote rules. Such scenario must be covered by test_update_rules_protocols.\n\nThis name should be something like test_update_rules_split\n\nhttps://testing.googleblog.com/2014/10/testing-on-toilet-writing-descriptive.html","commit_id":"f7be001b8fb004a7070b7a2e5fe9518f5861248e"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"80ef3c72709eb58262dce1bf5839a104e28f04c9","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        self.sg \u003d ovsfw.SecurityGroup(\u0027123\u0027)"},{"line_number":57,"context_line":"        self.sg.members \u003d {\u0027type\u0027: [1, 2, 3, 4]}"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_update_rules_no_protocols(self):"},{"line_number":60,"context_line":"        rules \u003d ["},{"line_number":61,"context_line":"            {\u0027foo\u0027: \u0027bar\u0027, \u0027rule\u0027: \u0027all\u0027}, {\u0027bar\u0027: \u0027foo\u0027},"},{"line_number":62,"context_line":"            {\u0027remote_group_id\u0027: \u0027123456\u0027, \u0027foo\u0027: \u0027bar\u0027}]"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f436f4f_c4933200","line":59,"range":{"start_line":59,"start_character":8,"end_line":59,"end_character":38},"in_reply_to":"9f436f4f_53527914","updated":"2017-08-18 01:06:44.000000000","message":"Done","commit_id":"f7be001b8fb004a7070b7a2e5fe9518f5861248e"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"9752177138140b180d4f13f830815fcd95e7db78","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                          constants.PROTO_NUM_TCP,"},{"line_number":88,"context_line":"                          94,"},{"line_number":89,"context_line":"                          \u0027baz\u0027], protos)"},{"line_number":90,"context_line":"        self.assertFalse(self.sg.remote_rules)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def test_get_ethertype_filtered_addresses(self):"},{"line_number":93,"context_line":"        addresses \u003d self.sg.get_ethertype_filtered_addresses(\u0027type\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f436f4f_336ed5c8","line":90,"range":{"start_line":90,"start_character":0,"end_line":90,"end_character":46},"updated":"2017-08-17 11:33:40.000000000","message":"This doesn\u0027t belong here, this method aims to test protocols normalization.","commit_id":"f7be001b8fb004a7070b7a2e5fe9518f5861248e"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"80ef3c72709eb58262dce1bf5839a104e28f04c9","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                          constants.PROTO_NUM_TCP,"},{"line_number":88,"context_line":"                          94,"},{"line_number":89,"context_line":"                          \u0027baz\u0027], protos)"},{"line_number":90,"context_line":"        self.assertFalse(self.sg.remote_rules)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def test_get_ethertype_filtered_addresses(self):"},{"line_number":93,"context_line":"        addresses \u003d self.sg.get_ethertype_filtered_addresses(\u0027type\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f436f4f_a466ee28","line":90,"range":{"start_line":90,"start_character":0,"end_line":90,"end_character":46},"in_reply_to":"9f436f4f_336ed5c8","updated":"2017-08-18 01:06:44.000000000","message":"Done","commit_id":"f7be001b8fb004a7070b7a2e5fe9518f5861248e"}]}