)]}'
{"dragonflow/controller/common/utils.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"abd9c3dd23a8a88cad4fb60b39102c70b0e9872c","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"def ipv4_text_to_int(ip_text):"},{"line_number":24,"context_line":"    if not ip_text:"},{"line_number":25,"context_line":"        return 0"},{"line_number":26,"context_line":"    assert isinstance(ip_text, str)"},{"line_number":27,"context_line":"    return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_69e251d7","line":25,"range":{"start_line":24,"start_character":0,"end_line":25,"end_character":16},"updated":"2016-10-27 13:46:51.000000000","message":"according to the context, the ip_text should be the string of ipv4 address. I think it is not good to hide \"\", None, or even 0 here and let the consuming code to handle it.\n\nI think it would be nice to throw exception here if the ip text is not valid, because we are expecting \"ipv4 text\" here.","commit_id":"2d2bd90c2768b8d731b3ce2e8bdcc02badc694d9"},{"author":{"_account_id":20287,"name":"hujie","email":"mike.hu@huawei.com","username":"hujie"},"change_message_id":"4b98e87975cc89189e6d9e0354fb50dd9c764dca","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"def ipv4_text_to_int(ip_text):"},{"line_number":24,"context_line":"    if not ip_text:"},{"line_number":25,"context_line":"        return 0"},{"line_number":26,"context_line":"    assert isinstance(ip_text, str)"},{"line_number":27,"context_line":"    return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_7a4da371","line":25,"range":{"start_line":24,"start_character":0,"end_line":25,"end_character":16},"in_reply_to":"ba5da102_69e251d7","updated":"2016-10-28 02:16:25.000000000","message":"ok, make sense","commit_id":"2d2bd90c2768b8d731b3ce2e8bdcc02badc694d9"},{"author":{"_account_id":21007,"name":"yuan wei","email":"wei.yuan@huawei.com","username":"yuanwei"},"change_message_id":"954860f30feb15e8ce4eb7671b6c9d6398fb6abc","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def ipv4_text_to_int(ip_text):"},{"line_number":25,"context_line":"    if not ip_text:"},{"line_number":26,"context_line":"        raise exceptions.InvalidIPAddressException()"},{"line_number":27,"context_line":"    assert isinstance(ip_text, str)"},{"line_number":28,"context_line":"    return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_3255f64f","line":26,"updated":"2016-10-29 07:59:38.000000000","message":"A more widely valid check will be done in calling \u0027addrconv.ipv4.text_to_bin\u0027, why to add this simple check here to throw a exception? If it must use DF own exception type, we could catch the exceptions thrown when calling \u0027addrconv.ipv4.text_to_bin\u0027 and then throw our own type exception.","commit_id":"a2e5ecd69bfd32daba36feec9dd7f453615ab41a"},{"author":{"_account_id":20287,"name":"hujie","email":"mike.hu@huawei.com","username":"hujie"},"change_message_id":"61e0fe10ce2e1ae4c6d1ed30b6b5d05f43e38178","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def ipv4_text_to_int(ip_text):"},{"line_number":25,"context_line":"    if not ip_text:"},{"line_number":26,"context_line":"        raise exceptions.InvalidIPAddressException()"},{"line_number":27,"context_line":"    assert isinstance(ip_text, str)"},{"line_number":28,"context_line":"    return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_b231060e","line":26,"in_reply_to":"ba5da102_3255f64f","updated":"2016-10-29 08:42:21.000000000","message":"Done","commit_id":"a2e5ecd69bfd32daba36feec9dd7f453615ab41a"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"09424a85e3199a382c7f5ac569ac9843b63021d9","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    try:"},{"line_number":26,"context_line":"        return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":27,"context_line":"    except Exception:"},{"line_number":28,"context_line":"        raise exceptions.InvalidIPAddressException()"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def mask_ntob(mask, err_msg\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_19cbb2a6","line":28,"range":{"start_line":28,"start_character":0,"end_line":28,"end_character":52},"updated":"2016-10-31 03:45:46.000000000","message":"The new exception is hiding the valued information(ip_text) of actual error. It will make the log hard to read.\n\nBesides, you remove the string check(assert isinstance(ip_text, str))\n\n\nI think we can check if it is a string be passed in. And then just return struct.unpack. The original exception should be fine to tell what\u0027t wrong.","commit_id":"216e5e9ba72862463347e3059f61a0850595d9d9"},{"author":{"_account_id":20287,"name":"hujie","email":"mike.hu@huawei.com","username":"hujie"},"change_message_id":"04285472ace7adaa87948df91c54cb23e904ed01","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    try:"},{"line_number":26,"context_line":"        return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":27,"context_line":"    except Exception:"},{"line_number":28,"context_line":"        raise exceptions.InvalidIPAddressException()"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def mask_ntob(mask, err_msg\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_bc2b9493","line":28,"range":{"start_line":28,"start_character":0,"end_line":28,"end_character":52},"in_reply_to":"ba5da102_19cbb2a6","updated":"2016-10-31 07:41:51.000000000","message":"addrconv.ipv4.text_to_bin could deal with string and also the integer type, so I think assert ip_text is string is unnecessary and unfitable, if the ip_text is None, \"\", 1, 100, \"10.10.10.\" or \"123-456\", all the invalid ip address could be accept by addrconv.ipv4.text_to_bin and raise exception, this function is used to parse the input ip and return the value we wanted in Openflow, if it raise exception, I think we could call the exception InvalidIPAddressException.","commit_id":"216e5e9ba72862463347e3059f61a0850595d9d9"},{"author":{"_account_id":20287,"name":"hujie","email":"mike.hu@huawei.com","username":"hujie"},"change_message_id":"4268b068cb47c7ae87e7bf3d4a7aff5dfae65ab0","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    try:"},{"line_number":26,"context_line":"        return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":27,"context_line":"    except Exception:"},{"line_number":28,"context_line":"        raise exceptions.InvalidIPAddressException()"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def mask_ntob(mask, err_msg\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_92e87712","line":28,"range":{"start_line":28,"start_character":0,"end_line":28,"end_character":52},"in_reply_to":"ba5da102_8fabf41c","updated":"2016-10-31 08:49:10.000000000","message":"ok, I got you, I can add the ip print in the log msg.","commit_id":"216e5e9ba72862463347e3059f61a0850595d9d9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"2fe84f1d93b1c9a08369541c1e16005477b5b2bf","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    try:"},{"line_number":26,"context_line":"        return struct.unpack(\u0027!I\u0027, addrconv.ipv4.text_to_bin(ip_text))[0]"},{"line_number":27,"context_line":"    except Exception:"},{"line_number":28,"context_line":"        raise exceptions.InvalidIPAddressException()"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def mask_ntob(mask, err_msg\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_8fabf41c","line":28,"range":{"start_line":28,"start_character":0,"end_line":28,"end_character":52},"in_reply_to":"ba5da102_bc2b9493","updated":"2016-10-31 08:40:52.000000000","message":"If you are trying to extend the functionality of this function to accept int value, then I think the function name needs to change to ipv4_addr_to_int(or something like this). Or else, it will cause confusion to the user of it, as it said ipv4 text, while an integer is also acceptable.\n\n\nThe concern I have here, is that if you just InvalidIPAddressException, we don\u0027t know what ip is invalid. It is barely helpful, if this exception appears in log. If we just use the exception from addrconv.ipv4.text_to_bin, it at least tells what is going wrong.","commit_id":"216e5e9ba72862463347e3059f61a0850595d9d9"}]}