)]}'
{"neutronclient/neutron/v2_0/subnet.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"949e960306f5bc8ccb6a822e311982ad4fc3af6f","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            help\u003d_(\u0027Network ID or name this subnet belongs to.\u0027))"},{"line_number":177,"context_line":"        parser.add_argument("},{"line_number":178,"context_line":"            \u0027cidr\u0027, metavar\u003d\u0027CIDR/PREFIXLEN\u0027,"},{"line_number":179,"context_line":"            help\u003d_(\u0027CIDR or prefix length of subnet to create.\u0027))"},{"line_number":180,"context_line":"        parser.add_argument("},{"line_number":181,"context_line":"            \u0027--ipv6-ra-mode\u0027,"},{"line_number":182,"context_line":"            choices\u003d[\u0027dhcpv6-stateful\u0027, \u0027dhcpv6-stateless\u0027, \u0027slaac\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_b099f670","line":179,"updated":"2015-04-13 15:43:54.000000000","message":"This strikes me as a poor use model.  The follow doesn\u0027t make any sense to me but would be allowed by this.\n\n  --cidr 24","commit_id":"9ff7c47cd0e2e4ea7b682087b1b961be10119d42"}],"neutronclient/shell.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        values_specs \u003d sub_argv[index:]"},{"line_number":100,"context_line":"    known_args, _values_specs \u003d cmd_parser.parse_known_args(_argv)"},{"line_number":101,"context_line":"    if isinstance(cmd, subnet.CreateSubnet) and not known_args.cidr:"},{"line_number":102,"context_line":"        _values_specs, cidr \u003d parse_cidr(_values_specs)"},{"line_number":103,"context_line":"        if cidr:"},{"line_number":104,"context_line":"            known_args.cidr \u003d cidr"},{"line_number":105,"context_line":"    cmd.values_specs \u003d (index \u003d\u003d -1 and _values_specs or values_specs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_4090d095","line":102,"updated":"2015-04-15 18:53:00.000000000","message":"Assuming you add the is_valid_cidr utility that I suggest on L120 and assuming that we only consider the first argument (L115), then this code boils down to this:\n\n  if (isinstance(cmd, subnet.CreateSubnet)\n          and not known_args.cidr\n          and _values_specs\n          and utils.is_valid_cidr(_values_specs[0])):\n      known_args.cidr \u003d _values_specs.pop(0)","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":12524,"name":"Zengfa Gao","email":"zengfa.gao@hp.com","username":"zengfagao"},"change_message_id":"d1f574d02e3bcf6aeebc5915f12a537e15c8033b","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        values_specs \u003d sub_argv[index:]"},{"line_number":100,"context_line":"    known_args, _values_specs \u003d cmd_parser.parse_known_args(_argv)"},{"line_number":101,"context_line":"    if isinstance(cmd, subnet.CreateSubnet) and not known_args.cidr:"},{"line_number":102,"context_line":"        _values_specs, cidr \u003d parse_cidr(_values_specs)"},{"line_number":103,"context_line":"        if cidr:"},{"line_number":104,"context_line":"            known_args.cidr \u003d cidr"},{"line_number":105,"context_line":"    cmd.values_specs \u003d (index \u003d\u003d -1 and _values_specs or values_specs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_80ca584e","line":102,"in_reply_to":"5aa745d1_4090d095","updated":"2015-04-15 19:03:34.000000000","message":"It may not be the first item.","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # to be separated from previous positional parameter."},{"line_number":112,"context_line":"    # when cidr was separated from network, the value will not be able"},{"line_number":113,"context_line":"    # to be parsed into known_args"},{"line_number":114,"context_line":"    cidr \u003d None"},{"line_number":115,"context_line":"    for value in value_specs:"},{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            IPNetwork(value)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_208dc47b","line":114,"updated":"2015-04-15 18:53:00.000000000","message":"style nit:  Don\u0027t need this variable.  See below.","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    # when cidr was separated from network, the value will not be able"},{"line_number":113,"context_line":"    # to be parsed into known_args"},{"line_number":114,"context_line":"    cidr \u003d None"},{"line_number":115,"context_line":"    for value in value_specs:"},{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            IPNetwork(value)"},{"line_number":118,"context_line":"            value_specs.remove(value)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_c0f2a006","line":115,"updated":"2015-04-15 18:53:00.000000000","message":"Instead of iterating, can/should we assume that the cidr will be the first argument if it is there at all?","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    cidr \u003d None"},{"line_number":115,"context_line":"    for value in value_specs:"},{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            IPNetwork(value)"},{"line_number":118,"context_line":"            value_specs.remove(value)"},{"line_number":119,"context_line":"            cidr \u003d value"},{"line_number":120,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_a0f794f4","line":117,"updated":"2015-04-15 18:53:00.000000000","message":"netaddr.IPNetwork(value)","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":12524,"name":"Zengfa Gao","email":"zengfa.gao@hp.com","username":"zengfagao"},"change_message_id":"d1f574d02e3bcf6aeebc5915f12a537e15c8033b","unresolved":false,"context_lines":[{"line_number":114,"context_line":"    cidr \u003d None"},{"line_number":115,"context_line":"    for value in value_specs:"},{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            IPNetwork(value)"},{"line_number":118,"context_line":"            value_specs.remove(value)"},{"line_number":119,"context_line":"            cidr \u003d value"},{"line_number":120,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_20ef04b2","line":117,"in_reply_to":"5aa745d1_a0f794f4","updated":"2015-04-15 19:03:34.000000000","message":"Done","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    for value in value_specs:"},{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            IPNetwork(value)"},{"line_number":118,"context_line":"            value_specs.remove(value)"},{"line_number":119,"context_line":"            cidr \u003d value"},{"line_number":120,"context_line":"        except Exception:"},{"line_number":121,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_60daac7a","line":118,"updated":"2015-04-15 18:53:00.000000000","message":"Removing an item while iterating a list isn\u0027t good","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        try:"},{"line_number":117,"context_line":"            IPNetwork(value)"},{"line_number":118,"context_line":"            value_specs.remove(value)"},{"line_number":119,"context_line":"            cidr \u003d value"},{"line_number":120,"context_line":"        except Exception:"},{"line_number":121,"context_line":"            pass"},{"line_number":122,"context_line":"    return value_specs, cidr"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_c07bc07a","line":119,"updated":"2015-04-15 18:53:00.000000000","message":"style nit:  Just return here instead of saving the value and returning later.","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            IPNetwork(value)"},{"line_number":118,"context_line":"            value_specs.remove(value)"},{"line_number":119,"context_line":"            cidr \u003d value"},{"line_number":120,"context_line":"        except Exception:"},{"line_number":121,"context_line":"            pass"},{"line_number":122,"context_line":"    return value_specs, cidr"},{"line_number":123,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_0082c852","line":120,"updated":"2015-04-15 18:53:00.000000000","message":"Catching Exception is a bit of a hack.  Though, I\u0027m not sure how to do it better.  Could we at least capture this in a utility (if there isn\u0027t already one) to isolate this yucky part?\n\ndef is_valid_cidr(cidr):\n    try:\n        netaddr.IPNetwork(cidr)\n        return True\n    except:\n        return False","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            value_specs.remove(value)"},{"line_number":119,"context_line":"            cidr \u003d value"},{"line_number":120,"context_line":"        except Exception:"},{"line_number":121,"context_line":"            pass"},{"line_number":122,"context_line":"    return value_specs, cidr"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_60aceced","line":121,"updated":"2015-04-15 18:53:00.000000000","message":"return here","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4ea882fcc6c98d651710cfa6ed4eea23776e6e2","unresolved":false,"context_lines":[{"line_number":119,"context_line":"            cidr \u003d value"},{"line_number":120,"context_line":"        except Exception:"},{"line_number":121,"context_line":"            pass"},{"line_number":122,"context_line":"    return value_specs, cidr"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"def env(*_vars, **kwargs):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_40b9f02d","line":122,"updated":"2015-04-15 18:53:00.000000000","message":"Do you need to return value_specs here?  I don\u0027t think so and I think that returning a 2-tuple just complicates the method signature.","commit_id":"b5af8bb7b3f2d48824c11b5aead65a1244dc9d8a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfe6480fce265e06ad38e23457a516567290a49d","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    known_args, _values_specs \u003d cmd_parser.parse_known_args(_argv)"},{"line_number":100,"context_line":"    if(isinstance(cmd, subnet.CreateSubnet)"},{"line_number":101,"context_line":"       and not known_args.cidr"},{"line_number":102,"context_line":"       and _values_specs):"},{"line_number":103,"context_line":"        cidr \u003d parse_cidr(_values_specs)"},{"line_number":104,"context_line":"        known_args.cidr \u003d cidr"},{"line_number":105,"context_line":"        _values_specs.remove(cidr)"}],"source_content_type":"text/x-python","patch_set":10,"id":"dac4157f_6c56c8d3","line":102,"updated":"2015-04-21 17:55:30.000000000","message":"We shouldn\u0027t need to check _values_specs here in this boolean statement.  This could be cleaned up a bit like this:\n\nif isinstance(cmd, subnet.CreateSubnet) and not known_args.cidr:\n   cidr \u003d ...\n\n_value_specs will always be a list.  If it is empty, there is no harm because parse_cidr will return None and then my comment on L103 applies.","commit_id":"4a66c350f0d7c2c99e4909d0db1084c6f5d8e738"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfe6480fce265e06ad38e23457a516567290a49d","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    if(isinstance(cmd, subnet.CreateSubnet)"},{"line_number":101,"context_line":"       and not known_args.cidr"},{"line_number":102,"context_line":"       and _values_specs):"},{"line_number":103,"context_line":"        cidr \u003d parse_cidr(_values_specs)"},{"line_number":104,"context_line":"        known_args.cidr \u003d cidr"},{"line_number":105,"context_line":"        _values_specs.remove(cidr)"},{"line_number":106,"context_line":"    cmd.values_specs \u003d (index \u003d\u003d -1 and _values_specs or values_specs)"}],"source_content_type":"text/x-python","patch_set":10,"id":"dac4157f_0cd74c24","line":103,"updated":"2015-04-21 17:55:30.000000000","message":"Need to check cidr here before assigning it and trying to remove it.  In addition to the empty list case that I mentioned on L102, it could return None because no valid cidr is found in the remaining arguments.  It should be handled either way.\n\n  if isinstance(cmd, subnet.CreateSubnet) and not known_args.cidr:\n      cidr \u003d parse_cidr(_values_specs)\n      if cidr:\n          known_args.cidr \u003d cidr\n          _values_specs.remove(cidr)","commit_id":"4a66c350f0d7c2c99e4909d0db1084c6f5d8e738"},{"author":{"_account_id":12524,"name":"Zengfa Gao","email":"zengfa.gao@hp.com","username":"zengfagao"},"change_message_id":"08805d2576a0c23713abdb35dec1be355b4ac020","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    if(isinstance(cmd, subnet.CreateSubnet)"},{"line_number":101,"context_line":"       and not known_args.cidr"},{"line_number":102,"context_line":"       and _values_specs):"},{"line_number":103,"context_line":"        cidr \u003d parse_cidr(_values_specs)"},{"line_number":104,"context_line":"        known_args.cidr \u003d cidr"},{"line_number":105,"context_line":"        _values_specs.remove(cidr)"},{"line_number":106,"context_line":"    cmd.values_specs \u003d (index \u003d\u003d -1 and _values_specs or values_specs)"}],"source_content_type":"text/x-python","patch_set":10,"id":"dac4157f_8c5a3c4f","line":103,"in_reply_to":"dac4157f_0cd74c24","updated":"2015-04-21 18:02:51.000000000","message":"Done","commit_id":"4a66c350f0d7c2c99e4909d0db1084c6f5d8e738"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfe6480fce265e06ad38e23457a516567290a49d","unresolved":false,"context_lines":[{"line_number":107,"context_line":"    return cmd.run(known_args)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"def parse_cidr(value_specs):"},{"line_number":111,"context_line":"    # Bug 1442771, argparse does not allow optional positional parameter"},{"line_number":112,"context_line":"    # to be separated from previous positional parameter."},{"line_number":113,"context_line":"    # When cidr was separated from network, the value will not be able"}],"source_content_type":"text/x-python","patch_set":10,"id":"dac4157f_4c5e6413","line":110,"updated":"2015-04-21 17:55:30.000000000","message":"nit:  How about \" get_first_valid_cidr\" as a name.  It seems to better describe what the method does.","commit_id":"4a66c350f0d7c2c99e4909d0db1084c6f5d8e738"}]}