)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14119,"name":"Sam Yaple","email":"sam+git@yaple.net","username":"SamYaple"},"change_message_id":"8b80edb90788024d7af8d108bf82eac7d43de9cb","unresolved":false,"context_lines":[{"line_number":12,"context_line":"does so."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ie1a67ea1afd69d5de4326f9e9a7243362b912582"},{"line_number":15,"context_line":"Implements: ICMP fields validity checkup"},{"line_number":16,"context_line":"Closes-Bug: #1511940"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1a26ad4f_1e6b9fb8","line":15,"updated":"2015-11-02 07:21:17.000000000","message":"This field is for a blueprint only. Did you create a blueprint for this? If not, remove this line","commit_id":"f302eaa645132c44b79a899da03f5ee5471537ec"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"3c94e8550f48a796786084e3cb9e21c27ba6e3f4","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Added ICMP \u0027type\u0027 and \u0027code\u0027 checking capability to \u0027netutils\u0027 module"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"ICMP has fields \u0027type\u0027 and \u0027code\u0027 which is used while adding a security"},{"line_number":10,"context_line":"group rule using Horizon, so there \u0027type\u0027 and \u0027code\u0027 could be checked, for"},{"line_number":11,"context_line":"validity of \u0027type\u0027 and \u0027code\u0027 field, I have written two functions that"},{"line_number":12,"context_line":"does so."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa80f949_189fa604","line":9,"updated":"2015-11-04 04:42:50.000000000","message":"Consider changing to : ICMP has the \u0027type\u0027 and \u0027code\u0027 fields which are used while adding a security group rule via Horizon. I have written functions which validate these fields and added them to the \u0027netutils\u0027 module.","commit_id":"d229527f2984c8d6f591f02164982afe1162228c"}],"oslo_utils/netutils.py":[{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"3c94e8550f48a796786084e3cb9e21c27ba6e3f4","unresolved":false,"context_lines":[{"line_number":178,"context_line":"    return (val \u003e 0 and val \u003c\u003d 65535)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def is_valid_icmp_type(type):"},{"line_number":182,"context_line":"    \"\"\"Verify if ICMP type is valid."},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    :param type: ICMP *type* field"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_b84bfaaf","line":181,"updated":"2015-11-04 04:42:50.000000000","message":"Consider combining these functions, since you are essentially checking that a value is valid and within a certain range. For instance, something like is_valid(value,begin_range\u003d0,end_range\u003d255) with the logic that is defined below. Thoughts?","commit_id":"d229527f2984c8d6f591f02164982afe1162228c"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"92f3d29281a6944eac894ae7b1e0db98d6604f3e","unresolved":false,"context_lines":[{"line_number":186,"context_line":"    :param start: start number of range"},{"line_number":187,"context_line":"    :param end: end number of range"},{"line_number":188,"context_line":"    :returns: bool"},{"line_number":189,"context_line":"    \"\"\""},{"line_number":190,"context_line":"    try:"},{"line_number":191,"context_line":"        val \u003d int(value)"},{"line_number":192,"context_line":"    except (ValueError, TypeError):"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_3eeb24b2","line":189,"updated":"2015-11-05 19:00:16.000000000","message":"Maybe don\u0027t waste conversion call if already an int.\n\nHow about:\n\n    def _is_int_in_range(value, start, end):\n       if not isinstance(value, six.integer_types):\n           value \u003d int(value)\n       return value\n\nI also don\u0027t like returning False on invalid int, that doesn\u0027t quite seem to make sense, the caller should handle that IMHO, because its hard to know what to do in that case ... Maybe just do:\n\n    def _is_int_in_range(value, start, end):\n       if not isinstance(value, six.integer_types):\n           try:\n              value \u003d int(value)\n           except (ValueError, TypeError):\n              raise ValueError(\"Unable to convert %s to\"\n                               \" an integer\" % value)\n       return value","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"b31c44224ae10bca3338f982d4d50faf6d75768c","unresolved":false,"context_lines":[{"line_number":186,"context_line":"    :param start: start number of range"},{"line_number":187,"context_line":"    :param end: end number of range"},{"line_number":188,"context_line":"    :returns: bool"},{"line_number":189,"context_line":"    \"\"\""},{"line_number":190,"context_line":"    try:"},{"line_number":191,"context_line":"        val \u003d int(value)"},{"line_number":192,"context_line":"    except (ValueError, TypeError):"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_bce391a3","line":189,"in_reply_to":"fa80f949_3eeb24b2","updated":"2015-11-06 02:48:30.000000000","message":"Hmm. I can see what you mean, but this is a condition function which kind of leads you to expect a True or False.","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":18930,"name":"Suraj Deshmukh","email":"surajssd009005@gmail.com","username":"suraj.deshmukh"},"change_message_id":"66d2ba9e2839b5bb89872b9507ef51ce630e5aff","unresolved":false,"context_lines":[{"line_number":186,"context_line":"    :param start: start number of range"},{"line_number":187,"context_line":"    :param end: end number of range"},{"line_number":188,"context_line":"    :returns: bool"},{"line_number":189,"context_line":"    \"\"\""},{"line_number":190,"context_line":"    try:"},{"line_number":191,"context_line":"        val \u003d int(value)"},{"line_number":192,"context_line":"    except (ValueError, TypeError):"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_8c1a187c","line":189,"in_reply_to":"fa80f949_3eeb24b2","updated":"2015-11-06 05:58:26.000000000","message":"The source calling this kind of function is at [1]. Where they expect either \u0027True\u0027 or \u0027False\u0027. And accordingly the exceptions are raised in that respective calling projects. Look at [1] where another function from \u0027oslo_utils.netutils\u0027 is called.\n\nThe other function of this kind is \u0027is_valid_port\u0027 in \u0027oslo_utils.netutils\u0027 which checks if the ports are in valid range of 1 to 65535.\n\nJoshua Harlow, your idea of using isinstance is great, but please look at [1] and [2] once.\n\n[1] https://github.com/openstack/horizon/blob/master/horizon/utils/validators.py#L26\n\n[2] https://github.com/openstack/glance/blob/master/glance/common/utils.py#L541","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"565d82606c816415856a4c50e887b5cb6a09973b","unresolved":false,"context_lines":[{"line_number":186,"context_line":"    :param start: start number of range"},{"line_number":187,"context_line":"    :param end: end number of range"},{"line_number":188,"context_line":"    :returns: bool"},{"line_number":189,"context_line":"    \"\"\""},{"line_number":190,"context_line":"    try:"},{"line_number":191,"context_line":"        val \u003d int(value)"},{"line_number":192,"context_line":"    except (ValueError, TypeError):"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_5feb7478","line":189,"in_reply_to":"fa80f949_8c1a187c","updated":"2015-11-06 06:35:10.000000000","message":"Gotcha, this api is weird, but anyway, point taken :-P","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"93f59dc64de6b964255ebd688d5d72d74cf77b12","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    :param type: ICMP *type* field"},{"line_number":201,"context_line":"    :returns: bool"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    ICMP *type* field can be of value from 0 to 255."},{"line_number":204,"context_line":"    \"\"\""},{"line_number":205,"context_line":"    return _is_int_in_range(type, 0, 255)"},{"line_number":206,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_03740733","line":203,"updated":"2015-11-04 13:07:18.000000000","message":"Consider: ICMP *type* field can only be a valid integer.\n\nYou do not have to list the range here, since the range is shown in the actual validation call.","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"93f59dc64de6b964255ebd688d5d72d74cf77b12","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    :param code: ICMP *code* field"},{"line_number":212,"context_line":"    :returns: bool"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    ICMP *code* field can have value either None or 0 to 255."},{"line_number":215,"context_line":"    \"\"\""},{"line_number":216,"context_line":"    if code is None:"},{"line_number":217,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_23cfcb80","line":214,"updated":"2015-11-04 13:07:18.000000000","message":"Consider: ICMP *code* field is optional (can be None) or a valid integer.\n\nYou do not have to list the range here, since the range is shown in the actual validation call.","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":18930,"name":"Suraj Deshmukh","email":"surajssd009005@gmail.com","username":"suraj.deshmukh"},"change_message_id":"9dfea8583c67c91219c8911af97531f14bca2604","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    :param code: ICMP *code* field"},{"line_number":212,"context_line":"    :returns: bool"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    ICMP *code* field can have value either None or 0 to 255."},{"line_number":215,"context_line":"    \"\"\""},{"line_number":216,"context_line":"    if code is None:"},{"line_number":217,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_6626416a","line":214,"in_reply_to":"fa80f949_23cfcb80","updated":"2015-11-04 13:58:05.000000000","message":"Wouldn\u0027t it be good if somebody just see docstring and there he/she could find what is the range of type or code. Though redundant its good to have it in documentation as docstring. What do you say?","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"b626d0d96471605c6604e0a8f951ee8c3e20b117","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    :param code: ICMP *code* field"},{"line_number":212,"context_line":"    :returns: bool"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    ICMP *code* field can have value either None or 0 to 255."},{"line_number":215,"context_line":"    \"\"\""},{"line_number":216,"context_line":"    if code is None:"},{"line_number":217,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa80f949_4778bec4","line":214,"in_reply_to":"fa80f949_6626416a","updated":"2015-11-05 02:53:38.000000000","message":"That is a good point. How about a compromise with \u0027... or a valid integer having a value of  0 up to and including 255\u0027 ?\n\nThis also applies to the comment I added to the icmp type validation function.","commit_id":"6afac1c8f5c5a47f6b1fc17286faa8d61709dc46"},{"author":{"_account_id":9796,"name":"ChangBo Guo","email":"glongwave@gmail.com","username":"gcb"},"change_message_id":"86b7aa76529a10f54a36116cb6d44d94eaea3d9f","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def _is_int_in_range(value, start, end):"},{"line_number":182,"context_line":"    \"\"\"Try to convert value to int and check if it lies within"},{"line_number":183,"context_line":"    range \u0027start\u0027 to \u0027end\u0027"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    :param value: value to verify"},{"line_number":186,"context_line":"    :param start: start number of range"}],"source_content_type":"text/x-python","patch_set":5,"id":"da85f559_f3739e7e","line":183,"updated":"2015-11-10 13:44:42.000000000","message":"nit:  need a period here.","commit_id":"418e14ca26449e01a7618ae25a32fbb9c673b3ed"}],"oslo_utils/tests/test_netutils.py":[{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"3c94e8550f48a796786084e3cb9e21c27ba6e3f4","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        for input_value in valid_inputs:"},{"line_number":205,"context_line":"            self.assertTrue(netutils.is_valid_icmp_type(input_value))"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_valid_icmp_type_fail(self):"},{"line_number":208,"context_line":"        invalid_inputs \u003d [-1, \u0027-1\u0027, 256, \u0027256\u0027, None, \u0027None\u0027, \u0027five\u0027]"},{"line_number":209,"context_line":"        for input_value in invalid_inputs:"},{"line_number":210,"context_line":"            self.assertFalse(netutils.is_valid_icmp_type(input_value))"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_f8d8a2f1","line":207,"updated":"2015-11-04 04:42:50.000000000","message":"Consider renaming to test_invalid_icmp_type","commit_id":"d229527f2984c8d6f591f02164982afe1162228c"},{"author":{"_account_id":5164,"name":"jsimms","email":"justin.simms@gmail.com","username":"jsimms"},"change_message_id":"3c94e8550f48a796786084e3cb9e21c27ba6e3f4","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        for input_value in valid_inputs:"},{"line_number":215,"context_line":"            self.assertTrue(netutils.is_valid_icmp_code(input_value))"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def test_valid_icmp_code_fail(self):"},{"line_number":218,"context_line":"        invalid_inputs \u003d [-1, \u0027-1\u0027, 256, \u0027256\u0027, \u0027None\u0027, \u0027zero\u0027]"},{"line_number":219,"context_line":"        for input_value in invalid_inputs:"},{"line_number":220,"context_line":"            self.assertFalse(netutils.is_valid_icmp_code(input_value))"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_18e8067c","line":217,"updated":"2015-11-04 04:42:50.000000000","message":"Consider renaming to test_invalid_icmp_code","commit_id":"d229527f2984c8d6f591f02164982afe1162228c"}]}