)]}'
{"neutronclient/neutron/v2_0/vpn/ipsec_site_connection.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"d2905baa0dbc51bb2e77024686ecc5cdb2acf0fe","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            required\u003dTrue,"},{"line_number":110,"context_line":"            help\u003d_(\u0027Pre-shared key string.\u0027))"},{"line_number":111,"context_line":"        parser.add_argument("},{"line_number":112,"context_line":"            \u0027--local-ep-group\u0027, dest\u003d\u0027local_ep_group_id\u0027,"},{"line_number":113,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":114,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":115,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_1a41d60b","line":112,"range":{"start_line":112,"start_character":38,"end_line":112,"end_character":55},"updated":"2015-10-06 11:33:51.000000000","message":"we accpet both name and ID, so \"dest\" needs to be GROUP/EP_GROUP/LOCAL_EP_GROUP. I prefer to GROUP or EP_GROUP because the option name describes a lot.","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"22c8b2ebd1b96739f7bd31adfcae469701bf5fbc","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            required\u003dTrue,"},{"line_number":110,"context_line":"            help\u003d_(\u0027Pre-shared key string.\u0027))"},{"line_number":111,"context_line":"        parser.add_argument("},{"line_number":112,"context_line":"            \u0027--local-ep-group\u0027, dest\u003d\u0027local_ep_group_id\u0027,"},{"line_number":113,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":114,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":115,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_bd6d0a06","line":112,"in_reply_to":"7a2fa921_1a41d60b","updated":"2015-10-07 11:09:35.000000000","message":"Yeah, I named it the same as it is named on the server side (this is the final name that is used for the endpoint group).  I can change to local_ep_group (since there is a peer one too).","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"d2905baa0dbc51bb2e77024686ecc5cdb2acf0fe","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":114,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":115,"context_line":"        parser.add_argument("},{"line_number":116,"context_line":"            \u0027--peer-ep-group\u0027, dest\u003d\u0027peer_ep_group_id\u0027,"},{"line_number":117,"context_line":"            help\u003d_(\u0027Peer endpoint group ID/name with CIDR(s) for \u0027"},{"line_number":118,"context_line":"                   \u0027IPsec connection.\u0027))"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_7a632ab2","line":116,"updated":"2015-10-06 11:33:51.000000000","message":"Same as the above.","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"22c8b2ebd1b96739f7bd31adfcae469701bf5fbc","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":114,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":115,"context_line":"        parser.add_argument("},{"line_number":116,"context_line":"            \u0027--peer-ep-group\u0027, dest\u003d\u0027peer_ep_group_id\u0027,"},{"line_number":117,"context_line":"            help\u003d_(\u0027Peer endpoint group ID/name with CIDR(s) for \u0027"},{"line_number":118,"context_line":"                   \u0027IPsec connection.\u0027))"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_dd7006df","line":116,"in_reply_to":"7a2fa921_7a632ab2","updated":"2015-10-07 11:09:35.000000000","message":"Will update","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":14064,"name":"Kahou Lei","email":"kahou82@gmail.com","username":"kahou82"},"change_message_id":"515f1dfab78281c301ae0fe02dfd4965f9dc5821","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            message \u003d _(\"Must specify both local and peer endpoint groups.\")"},{"line_number":137,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":138,"context_line":"        if parsed_args.peer_cidrs and parsed_args.local_ep_group_id:"},{"line_number":139,"context_line":"            message \u003d _(\"Cannot specify both endoint groups and peer CIDR(s).\")"},{"line_number":140,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":141,"context_line":"        if not parsed_args.peer_cidrs and not parsed_args.local_ep_group_id:"},{"line_number":142,"context_line":"            message \u003d _(\"You must specify endpoint groups or peer CIDR(s).\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_d086ebb8","line":139,"updated":"2015-10-06 06:33:08.000000000","message":"Typo? endpoint instead of edoint?","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"7a0aeef0b95b79e409aef95ec50d22484c1c019c","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            message \u003d _(\"Must specify both local and peer endpoint groups.\")"},{"line_number":137,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":138,"context_line":"        if parsed_args.peer_cidrs and parsed_args.local_ep_group_id:"},{"line_number":139,"context_line":"            message \u003d _(\"Cannot specify both endoint groups and peer CIDR(s).\")"},{"line_number":140,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":141,"context_line":"        if not parsed_args.peer_cidrs and not parsed_args.local_ep_group_id:"},{"line_number":142,"context_line":"            message \u003d _(\"You must specify endpoint groups or peer CIDR(s).\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_c47e23ae","line":139,"in_reply_to":"7a2fa921_d086ebb8","updated":"2015-10-06 11:10:42.000000000","message":"Yeah, thanks!","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"d2905baa0dbc51bb2e77024686ecc5cdb2acf0fe","unresolved":false,"context_lines":[{"line_number":209,"context_line":"            _peer_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":210,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":211,"context_line":"                parsed_args.peer_ep_group_id)"},{"line_number":212,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":213,"context_line":"        return {\u0027ipsec_site_connection\u0027: body}"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_da203e4c","line":212,"updated":"2015-10-06 11:33:51.000000000","message":"Create and Update classes have a lot of duplicates.\nIt is better to have a common class to define common arguments and args2body.","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"22c8b2ebd1b96739f7bd31adfcae469701bf5fbc","unresolved":false,"context_lines":[{"line_number":209,"context_line":"            _peer_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":210,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":211,"context_line":"                parsed_args.peer_ep_group_id)"},{"line_number":212,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":213,"context_line":"        return {\u0027ipsec_site_connection\u0027: body}"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_7d78d2c4","line":212,"in_reply_to":"7a2fa921_da203e4c","updated":"2015-10-07 11:09:35.000000000","message":"Will do.","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"3cde319cc1e23a9699927c19685cb716ee6799a6","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            type\u003dutils.str2dict,"},{"line_number":58,"context_line":"            help\u003dvpn_utils.dpd_help(\"IPsec connection.\"))"},{"line_number":59,"context_line":"        parser.add_argument("},{"line_number":60,"context_line":"            \u0027--local-ep-group\u0027, dest\u003d\u0027local_ep_group\u0027,"},{"line_number":61,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":62,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":63,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_239d2d0a","line":60,"updated":"2015-10-08 07:15:15.000000000","message":"I think \"dest\u003d\u0027local_ep_group\u0027\" is un-necessary. By default, dest will be \u0027local_ep_group\u0027.","commit_id":"663a2f7544895625b1e49d1a6dc12eafde469ff4"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"d044da621d949d0c322d205463da82ccf819b5dc","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            type\u003dutils.str2dict,"},{"line_number":58,"context_line":"            help\u003dvpn_utils.dpd_help(\"IPsec connection.\"))"},{"line_number":59,"context_line":"        parser.add_argument("},{"line_number":60,"context_line":"            \u0027--local-ep-group\u0027, dest\u003d\u0027local_ep_group\u0027,"},{"line_number":61,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":62,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":63,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_4eb40581","line":60,"in_reply_to":"7a2fa921_239d2d0a","updated":"2015-10-08 11:41:43.000000000","message":"Yeah, I had local_ep_group_id to match what server is expecting, but Akihiro mentioned about not calling it ID, since wthe client can take name, so I dropped the id. Will remove. Thanks.","commit_id":"663a2f7544895625b1e49d1a6dc12eafde469ff4"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"3cde319cc1e23a9699927c19685cb716ee6799a6","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":62,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":63,"context_line":"        parser.add_argument("},{"line_number":64,"context_line":"            \u0027--peer-ep-group\u0027, dest\u003d\u0027peer_ep_group\u0027,"},{"line_number":65,"context_line":"            help\u003d_(\u0027Peer endpoint group ID/name with CIDR(s) for \u0027"},{"line_number":66,"context_line":"                   \u0027IPsec connection.\u0027))"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_43a2a94c","line":64,"updated":"2015-10-08 07:15:15.000000000","message":"ditto.","commit_id":"663a2f7544895625b1e49d1a6dc12eafde469ff4"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"d044da621d949d0c322d205463da82ccf819b5dc","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":62,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":63,"context_line":"        parser.add_argument("},{"line_number":64,"context_line":"            \u0027--peer-ep-group\u0027, dest\u003d\u0027peer_ep_group\u0027,"},{"line_number":65,"context_line":"            help\u003d_(\u0027Peer endpoint group ID/name with CIDR(s) for \u0027"},{"line_number":66,"context_line":"                   \u0027IPsec connection.\u0027))"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_cea81523","line":64,"in_reply_to":"7a2fa921_43a2a94c","updated":"2015-10-08 11:41:43.000000000","message":"Thanks.","commit_id":"663a2f7544895625b1e49d1a6dc12eafde469ff4"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"3cde319cc1e23a9699927c19685cb716ee6799a6","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {\u0027ipsec_site_connection\u0027: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class CreateIPsecSiteConnection(IPsecSiteConnectionMixin,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_23420d69","line":86,"updated":"2015-10-08 07:15:15.000000000","message":"how about using self.resource instead of \u0027ipsec_site_connection\u0027?","commit_id":"663a2f7544895625b1e49d1a6dc12eafde469ff4"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"d044da621d949d0c322d205463da82ccf819b5dc","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {\u0027ipsec_site_connection\u0027: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class CreateIPsecSiteConnection(IPsecSiteConnectionMixin,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_ee7cf1b6","line":86,"in_reply_to":"7a2fa921_23420d69","updated":"2015-10-08 11:41:43.000000000","message":"Yeah, that is cleaner.","commit_id":"663a2f7544895625b1e49d1a6dc12eafde469ff4"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ebaada3fd5e3a9abbce86db2e71a252afcd85190","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class IPsecSiteConnectionMixin(object):"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":54,"context_line":"        parser.add_argument("},{"line_number":55,"context_line":"            \u0027--dpd\u0027,"},{"line_number":56,"context_line":"            metavar\u003d\"action\u003dACTION,interval\u003dINTERVAL,timeout\u003dTIMEOUT\","}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_adb0a3f5","line":53,"updated":"2015-10-14 06:44:18.000000000","message":"How about renaming it to \"additional_known_arguments\", this will make it consistent with \"additional_args2body\"","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"79406574257edf2a26fab41fd9829938b1e36582","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class IPsecSiteConnectionMixin(object):"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":54,"context_line":"        parser.add_argument("},{"line_number":55,"context_line":"            \u0027--dpd\u0027,"},{"line_number":56,"context_line":"            metavar\u003d\"action\u003dACTION,interval\u003dINTERVAL,timeout\u003dTIMEOUT\","}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_bf652046","line":53,"in_reply_to":"5a2ca52d_adb0a3f5","updated":"2015-10-14 11:54:15.000000000","message":"But that is not the intent of this function. This is adding known arguments, which is consistent will all the other APIs (you\u0027ll see that there are one of these for most APIs).\n\nadditional_args2body is a special method, so that some of the conditional args that are common to both create and update can be added, and supplements the normal args2body method that is used on many APIs.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2ff9ca923e1bc9ba3b0ae7f87392588902e441d1","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class IPsecSiteConnectionMixin(object):"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":54,"context_line":"        parser.add_argument("},{"line_number":55,"context_line":"            \u0027--dpd\u0027,"},{"line_number":56,"context_line":"            metavar\u003d\"action\u003dACTION,interval\u003dINTERVAL,timeout\u003dTIMEOUT\","}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_59c8e574","line":53,"in_reply_to":"5a2ca52d_bf652046","updated":"2015-11-08 14:55:31.000000000","message":"I think it is better to define the main methods add_known_arguments and args2body. It helps us we have multiple mix-in classes.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"6dc36b213a6a5855c8ed0c0bd9c2d07145aa8d82","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class IPsecSiteConnectionMixin(object):"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":54,"context_line":"        parser.add_argument("},{"line_number":55,"context_line":"            \u0027--dpd\u0027,"},{"line_number":56,"context_line":"            metavar\u003d\"action\u003dACTION,interval\u003dINTERVAL,timeout\u003dTIMEOUT\","}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_a0341bde","line":53,"in_reply_to":"da85f559_59c8e574","updated":"2015-11-09 12:07:56.000000000","message":"Done","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2ff9ca923e1bc9ba3b0ae7f87392588902e441d1","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        parser.add_argument("},{"line_number":60,"context_line":"            \u0027--local-ep-group\u0027,"},{"line_number":61,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":62,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":63,"context_line":"        parser.add_argument("},{"line_number":64,"context_line":"            \u0027--peer-ep-group\u0027,"},{"line_number":65,"context_line":"            help\u003d_(\u0027Peer endpoint group ID/name with CIDR(s) for \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_79c5a959","line":62,"updated":"2015-11-08 14:55:31.000000000","message":"IPSec and IPsec are both used. More consistent usage looks better. My vote is IPsec.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"6dc36b213a6a5855c8ed0c0bd9c2d07145aa8d82","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        parser.add_argument("},{"line_number":60,"context_line":"            \u0027--local-ep-group\u0027,"},{"line_number":61,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"},{"line_number":62,"context_line":"                   \u0027IPSec connection.\u0027))"},{"line_number":63,"context_line":"        parser.add_argument("},{"line_number":64,"context_line":"            \u0027--peer-ep-group\u0027,"},{"line_number":65,"context_line":"            help\u003d_(\u0027Peer endpoint group ID/name with CIDR(s) for \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_6531650b","line":62,"in_reply_to":"da85f559_79c5a959","updated":"2015-11-09 12:07:56.000000000","message":"Done","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ebaada3fd5e3a9abbce86db2e71a252afcd85190","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            vpn_utils.validate_dpd_dict(parsed_args.dpd)"},{"line_number":75,"context_line":"            body[\u0027dpd\u0027] \u003d parsed_args.dpd"},{"line_number":76,"context_line":"        if parsed_args.local_ep_group:"},{"line_number":77,"context_line":"            _local_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":78,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":79,"context_line":"                parsed_args.local_ep_group)"},{"line_number":80,"context_line":"            body[\u0027local_ep_group_id\u0027] \u003d _local_epg"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_adcf43af","line":77,"updated":"2015-10-14 06:44:18.000000000","message":"I think local assignment can be avoided. We directly assign the id to body[\u0027local_ep_group_id\u0027]","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"79406574257edf2a26fab41fd9829938b1e36582","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            vpn_utils.validate_dpd_dict(parsed_args.dpd)"},{"line_number":75,"context_line":"            body[\u0027dpd\u0027] \u003d parsed_args.dpd"},{"line_number":76,"context_line":"        if parsed_args.local_ep_group:"},{"line_number":77,"context_line":"            _local_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":78,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":79,"context_line":"                parsed_args.local_ep_group)"},{"line_number":80,"context_line":"            body[\u0027local_ep_group_id\u0027] \u003d _local_epg"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_df2fbc9f","line":77,"in_reply_to":"5a2ca52d_adcf43af","updated":"2015-10-14 11:54:15.000000000","message":"Not really. The endpoint group can be an ID or a name. This method call will convert the name to an ID, or just pass through, if already an ID.\n\nIMHO, it is clearer to save that result in a temp variable, rather than assigning the results of the method directly to body. I\u0027d like to keep it this way, which is consistent with how other name/ID inputs are handled.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ebaada3fd5e3a9abbce86db2e71a252afcd85190","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            _local_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":78,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":79,"context_line":"                parsed_args.local_ep_group)"},{"line_number":80,"context_line":"            body[\u0027local_ep_group_id\u0027] \u003d _local_epg"},{"line_number":81,"context_line":"        if parsed_args.peer_ep_group:"},{"line_number":82,"context_line":"            _peer_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_4dc54f8c","line":80,"updated":"2015-10-14 06:44:18.000000000","message":"ditto","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"79406574257edf2a26fab41fd9829938b1e36582","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            _local_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":78,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":79,"context_line":"                parsed_args.local_ep_group)"},{"line_number":80,"context_line":"            body[\u0027local_ep_group_id\u0027] \u003d _local_epg"},{"line_number":81,"context_line":"        if parsed_args.peer_ep_group:"},{"line_number":82,"context_line":"            _peer_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_bf1d2075","line":80,"in_reply_to":"5a2ca52d_4dc54f8c","updated":"2015-10-14 11:54:15.000000000","message":"same answer.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ebaada3fd5e3a9abbce86db2e71a252afcd85190","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            _peer_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {self.resource: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_6dc28b87","line":85,"updated":"2015-10-14 06:44:18.000000000","message":"ditto","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"79406574257edf2a26fab41fd9829938b1e36582","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            _peer_epg \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {self.resource: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_ba520e12","line":85,"in_reply_to":"5a2ca52d_6dc28b87","updated":"2015-10-14 11:54:15.000000000","message":"same response.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2ff9ca923e1bc9ba3b0ae7f87392588902e441d1","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {self.resource: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class CreateIPsecSiteConnection(IPsecSiteConnectionMixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_39a641c2","line":86,"updated":"2015-11-08 14:55:31.000000000","message":"Isn\u0027t it better to return \u0027body\u0027 itself? This method takes body as an argument.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"6dc36b213a6a5855c8ed0c0bd9c2d07145aa8d82","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {self.resource: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class CreateIPsecSiteConnection(IPsecSiteConnectionMixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_40144ff4","line":86,"in_reply_to":"da85f559_39a641c2","updated":"2015-11-09 12:07:56.000000000","message":"I think it is better to return the dict. Each args2body returns a dict, so if we are overriding this in the mixin, we should do the same return (otherwise we\u0027ll be duplicating the dict in each usage of this).","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"3d113567999ec6550da59d585dc4caf41e1799d2","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {self.resource: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class CreateIPsecSiteConnection(IPsecSiteConnectionMixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_e12b0481","line":86,"in_reply_to":"da85f559_40144ff4","updated":"2015-11-09 13:33:02.000000000","message":"It makes sense based on your point. What I thought is a bit different.\n\nI am okay with either, but let me share what I thought.\n\nMy case is where we have multiple additional_args2body() like this. It would be:\n\n  resource \u003d additional_args2body_xxx(parsed_args)\n  resource \u003d additional_args2body_yyy(parsed_args, resource[self.resource])\n  resource \u003d additional_args2body_zzz(parsed_args, resource[self.resource])\n  return resource\n\nIt looks redundant.\n\nIf we return \u0027body\u0027 itself, it would be:\n\n  body \u003d additional_args2body_xxx(parsed_args)\n  body \u003d additional_args2body_yyy(parsed_args, body)\n  body \u003d additional_args2body_zzz(parsed_args, body)\n  return {self.resource: body}","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"70d3789c73d4b2c1cee8908ebeb12ca237ba49d1","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {self.resource: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class CreateIPsecSiteConnection(IPsecSiteConnectionMixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_a9df71d4","line":86,"in_reply_to":"da85f559_64f722d4","updated":"2015-11-12 12:22:50.000000000","message":"Fair enough. Thanks for the clarification.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"ff2f3d5abf38113ba5c65be6a1d4dee74d5bdd35","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                self.get_client(), \u0027endpoint_group\u0027,"},{"line_number":84,"context_line":"                parsed_args.peer_ep_group)"},{"line_number":85,"context_line":"            body[\u0027peer_ep_group_id\u0027] \u003d _peer_epg"},{"line_number":86,"context_line":"        return {self.resource: body}"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class CreateIPsecSiteConnection(IPsecSiteConnectionMixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_64f722d4","line":86,"in_reply_to":"da85f559_e12b0481","updated":"2015-11-09 13:46:50.000000000","message":"Understood. I don\u0027t think we need that level of flexibility for this, as we\u0027re taking shared arguments that apply to both create and update, with the rest in create\u0027s class.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2ff9ca923e1bc9ba3b0ae7f87392588902e441d1","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        parser.add_argument("},{"line_number":136,"context_line":"            \u0027--peer-cidr\u0027,"},{"line_number":137,"context_line":"            action\u003d\u0027append\u0027, dest\u003d\u0027peer_cidrs\u0027,"},{"line_number":138,"context_line":"            help\u003d_(\u0027[DEPRECATED] Remote subnet(s) in CIDR format. Cannot be \u0027"},{"line_number":139,"context_line":"                   \u0027specified when using endpoint groups. Only applicable, \u0027"},{"line_number":140,"context_line":"                   \u0027if subnet provided for VPN service.\u0027))"},{"line_number":141,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_9995ed41","line":138,"updated":"2015-11-08 14:55:31.000000000","message":"\"DEPRECATED in Mitaka\" looks better.\nneutronclient can be used with older releases.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"6dc36b213a6a5855c8ed0c0bd9c2d07145aa8d82","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        parser.add_argument("},{"line_number":136,"context_line":"            \u0027--peer-cidr\u0027,"},{"line_number":137,"context_line":"            action\u003d\u0027append\u0027, dest\u003d\u0027peer_cidrs\u0027,"},{"line_number":138,"context_line":"            help\u003d_(\u0027[DEPRECATED] Remote subnet(s) in CIDR format. Cannot be \u0027"},{"line_number":139,"context_line":"                   \u0027specified when using endpoint groups. Only applicable, \u0027"},{"line_number":140,"context_line":"                   \u0027if subnet provided for VPN service.\u0027))"},{"line_number":141,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_e013035b","line":138,"in_reply_to":"da85f559_9995ed41","updated":"2015-11-09 12:07:56.000000000","message":"Done","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ebaada3fd5e3a9abbce86db2e71a252afcd85190","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                        \"groups.\")"},{"line_number":165,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":166,"context_line":"        if parsed_args.peer_cidrs and parsed_args.local_ep_group:"},{"line_number":167,"context_line":"            message \u003d _(\"You cannot specify both endpoint groups and peer \""},{"line_number":168,"context_line":"                        \"CIDR(s).\")"},{"line_number":169,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":170,"context_line":"        if not parsed_args.peer_cidrs and not parsed_args.local_ep_group:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_edd8db55","line":167,"updated":"2015-10-14 06:44:18.000000000","message":"AFAIU, endpoint groups can be either local or peer. How about mentioning \u0027local endpoint group\u0027 explicitly in the message?","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"79406574257edf2a26fab41fd9829938b1e36582","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                        \"groups.\")"},{"line_number":165,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":166,"context_line":"        if parsed_args.peer_cidrs and parsed_args.local_ep_group:"},{"line_number":167,"context_line":"            message \u003d _(\"You cannot specify both endpoint groups and peer \""},{"line_number":168,"context_line":"                        \"CIDR(s).\")"},{"line_number":169,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":170,"context_line":"        if not parsed_args.peer_cidrs and not parsed_args.local_ep_group:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_3a1efe0c","line":167,"in_reply_to":"5a2ca52d_edd8db55","updated":"2015-10-14 11:54:15.000000000","message":"No. It is not needed. L161-162 ensures that either both endpoints are specified or neither (if you use endpoints, you must provide both).\n\nL166 detects when the user is mixing legacy (peer CIDRs) with endpoint groups (because of L161-162, both of them), which is not permitted.\n\nSo here, there will be both endpoint groups or none. I think the message is appropriate - we don\u0027t need to explicitly say local and peer.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ebaada3fd5e3a9abbce86db2e71a252afcd85190","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                        \"CIDR(s).\")"},{"line_number":169,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":170,"context_line":"        if not parsed_args.peer_cidrs and not parsed_args.local_ep_group:"},{"line_number":171,"context_line":"            message \u003d _(\"You must specify endpoint groups or peer CIDR(s).\")"},{"line_number":172,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":173,"context_line":"        body \u003d {"},{"line_number":174,"context_line":"            \u0027vpnservice_id\u0027: _vpnservice_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_4de8efe2","line":171,"updated":"2015-10-14 06:44:18.000000000","message":"ditto","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"79406574257edf2a26fab41fd9829938b1e36582","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                        \"CIDR(s).\")"},{"line_number":169,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":170,"context_line":"        if not parsed_args.peer_cidrs and not parsed_args.local_ep_group:"},{"line_number":171,"context_line":"            message \u003d _(\"You must specify endpoint groups or peer CIDR(s).\")"},{"line_number":172,"context_line":"            raise exceptions.CommandError(message)"},{"line_number":173,"context_line":"        body \u003d {"},{"line_number":174,"context_line":"            \u0027vpnservice_id\u0027: _vpnservice_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_dac82a77","line":171,"in_reply_to":"5a2ca52d_4de8efe2","updated":"2015-10-14 11:54:15.000000000","message":"Same thing. Not needed. L161-162 ensures we have both or neither endpoint groups. This ensures that we have specified either legacy (peer CIDRs) or new (both endpoint groups).","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"011319c7299a075e28f979df1c0c267f824098de","unresolved":false,"context_lines":[{"line_number":55,"context_line":"            \u0027--dpd\u0027,"},{"line_number":56,"context_line":"            metavar\u003d\"action\u003dACTION,interval\u003dINTERVAL,timeout\u003dTIMEOUT\","},{"line_number":57,"context_line":"            type\u003dutils.str2dict,"},{"line_number":58,"context_line":"            help\u003dvpn_utils.dpd_help(\"IPsec connection.\"))"},{"line_number":59,"context_line":"        parser.add_argument("},{"line_number":60,"context_line":"            \u0027--local-ep-group\u0027,"},{"line_number":61,"context_line":"            help\u003d_(\u0027Local endpoint group ID/name with subnet(s) for \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"da85f559_a9dbe37a","line":58,"updated":"2015-11-12 15:24:18.000000000","message":"Nit: period makes help text look a little odd.","commit_id":"03e5e5f981e3d574aa21381d7cfbc5784ef5b175"}],"neutronclient/neutron/v2_0/vpn/vpnservice.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"d2905baa0dbc51bb2e77024686ecc5cdb2acf0fe","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    resource \u003d \u0027vpnservice\u0027"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def __init__(self, app, app_args):"},{"line_number":44,"context_line":"        super(CreateVPNService, self).__init__(app, app_args)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":47,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_1a7324b3","line":44,"updated":"2015-10-06 11:33:51.000000000","message":"Any reason we need this?","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"22c8b2ebd1b96739f7bd31adfcae469701bf5fbc","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    resource \u003d \u0027vpnservice\u0027"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def __init__(self, app, app_args):"},{"line_number":44,"context_line":"        super(CreateVPNService, self).__init__(app, app_args)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":47,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_bd3bcaea","line":44,"in_reply_to":"7a2fa921_1a7324b3","updated":"2015-10-07 11:09:35.000000000","message":"Oops. I was debugging and had added __init__ with set_trace(). Forgot to get rid of this after.","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"85fc86dc0fc85dc21c38989e5d49fe0111426f52","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            help\u003d_(\u0027Router unique identifier for the VPN service.\u0027))"},{"line_number":60,"context_line":"        parser.add_argument("},{"line_number":61,"context_line":"            \u0027subnet\u0027, nargs\u003d\u0027?\u0027, metavar\u003d\u0027SUBNET\u0027,"},{"line_number":62,"context_line":"            help\u003d_(\u0027Unique identifier for the local private network.\u0027))"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def args2body(self, parsed_args):"},{"line_number":65,"context_line":"        if parsed_args.subnet:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a2fa921_0788b921","line":62,"updated":"2015-10-05 19:26:36.000000000","message":"Probably should indicate (DEPRECATED) in the help here too.","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2ff9ca923e1bc9ba3b0ae7f87392588902e441d1","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    resource \u003d \u0027vpnservice\u0027"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def __init__(self, app, app_args):"},{"line_number":44,"context_line":"        super(CreateVPNService, self).__init__(app, app_args)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":47,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_b9a0f191","line":44,"updated":"2015-11-08 14:55:31.000000000","message":"It seems unnecessary.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"6dc36b213a6a5855c8ed0c0bd9c2d07145aa8d82","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    resource \u003d \u0027vpnservice\u0027"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def __init__(self, app, app_args):"},{"line_number":44,"context_line":"        super(CreateVPNService, self).__init__(app, app_args)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def add_known_arguments(self, parser):"},{"line_number":47,"context_line":"        parser.add_argument("}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_80d8f754","line":44,"in_reply_to":"da85f559_b9a0f191","updated":"2015-11-09 12:07:56.000000000","message":"Done","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"2ff9ca923e1bc9ba3b0ae7f87392588902e441d1","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            help\u003d_(\u0027Router unique identifier for the VPN service.\u0027))"},{"line_number":60,"context_line":"        parser.add_argument("},{"line_number":61,"context_line":"            \u0027subnet\u0027, nargs\u003d\u0027?\u0027, metavar\u003d\u0027SUBNET\u0027,"},{"line_number":62,"context_line":"            help\u003d_(\u0027(DEPRECATED) Unique identifier for the local private \u0027"},{"line_number":63,"context_line":"                   \u0027network.\u0027))"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def args2body(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_59ad4587","line":62,"updated":"2015-11-08 14:55:31.000000000","message":"\"Deprecated\" in ipsecsiteconnection.py uses [], but () is used here.\nLooks inconsistent.\n\nWhy do you need to drop \u0027Subnet\u0027 from the original help text?","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"3d113567999ec6550da59d585dc4caf41e1799d2","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            help\u003d_(\u0027Router unique identifier for the VPN service.\u0027))"},{"line_number":60,"context_line":"        parser.add_argument("},{"line_number":61,"context_line":"            \u0027subnet\u0027, nargs\u003d\u0027?\u0027, metavar\u003d\u0027SUBNET\u0027,"},{"line_number":62,"context_line":"            help\u003d_(\u0027(DEPRECATED) Unique identifier for the local private \u0027"},{"line_number":63,"context_line":"                   \u0027network.\u0027))"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def args2body(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_a1a41c57","line":62,"in_reply_to":"da85f559_003de72a","updated":"2015-11-09 13:33:02.000000000","message":"Yes. I understand the situation, but  on the other hand, I know not a small number of people use a latest neutronclient for older releases (including Juno or Kilo).\nThus I would like to keep the help text correct as possible as we can.\n\nAt least, I think \u0027unique identifier for the local private network\u0027 associates users with \u0027network UUID\u0027. It is completely wrong, so I think it is better to keep \u0027subnet\u0027 in the help text.\n\nThought?","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"6dc36b213a6a5855c8ed0c0bd9c2d07145aa8d82","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            help\u003d_(\u0027Router unique identifier for the VPN service.\u0027))"},{"line_number":60,"context_line":"        parser.add_argument("},{"line_number":61,"context_line":"            \u0027subnet\u0027, nargs\u003d\u0027?\u0027, metavar\u003d\u0027SUBNET\u0027,"},{"line_number":62,"context_line":"            help\u003d_(\u0027(DEPRECATED) Unique identifier for the local private \u0027"},{"line_number":63,"context_line":"                   \u0027network.\u0027))"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def args2body(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_003de72a","line":62,"in_reply_to":"da85f559_59ad4587","updated":"2015-11-09 12:07:56.000000000","message":"Absence of subnet in the VPN service API indicates that the new API is being used, where subnet(s) is(are) specified in an endpoint group that is included in the connection. We don\u0027t want people to use this option, when using the new API. We want to move them to using endpoints.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"ff2f3d5abf38113ba5c65be6a1d4dee74d5bdd35","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            help\u003d_(\u0027Router unique identifier for the VPN service.\u0027))"},{"line_number":60,"context_line":"        parser.add_argument("},{"line_number":61,"context_line":"            \u0027subnet\u0027, nargs\u003d\u0027?\u0027, metavar\u003d\u0027SUBNET\u0027,"},{"line_number":62,"context_line":"            help\u003d_(\u0027(DEPRECATED) Unique identifier for the local private \u0027"},{"line_number":63,"context_line":"                   \u0027network.\u0027))"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def args2body(self, parsed_args):"}],"source_content_type":"text/x-python","patch_set":6,"id":"da85f559_848246ff","line":62,"in_reply_to":"da85f559_a1a41c57","updated":"2015-11-09 13:46:50.000000000","message":"Saw this after I pushed. Will change to subnet.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ebaada3fd5e3a9abbce86db2e71a252afcd85190","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                   \u0027network.\u0027))"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def args2body(self, parsed_args):"},{"line_number":66,"context_line":"        if parsed_args.subnet:"},{"line_number":67,"context_line":"            _subnet_id \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":68,"context_line":"                self.get_client(), \u0027subnet\u0027, parsed_args.subnet)"},{"line_number":69,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_2dd9f321","line":66,"updated":"2015-10-14 06:44:18.000000000","message":"I can find \u0027subnet\u0027 is a mandatory param (L61). How it can be None? I cannot understand the scenario where the else part can hit. \n\nAm I missing anything?\n\nIf this param needs to be \u0027None\u0027 by default then we can mention it\u0027s default value while adding it as an argument (L60-L63)","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"79406574257edf2a26fab41fd9829938b1e36582","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                   \u0027network.\u0027))"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def args2body(self, parsed_args):"},{"line_number":66,"context_line":"        if parsed_args.subnet:"},{"line_number":67,"context_line":"            _subnet_id \u003d neutronv20.find_resourceid_by_name_or_id("},{"line_number":68,"context_line":"                self.get_client(), \u0027subnet\u0027, parsed_args.subnet)"},{"line_number":69,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2ca52d_5a6dfa02","line":66,"in_reply_to":"5a2ca52d_2dd9f321","updated":"2015-10-14 11:54:15.000000000","message":"The subnet parameter used to be required. With the endpoint group feature, this is now deprecated and should not be specified, unless the user is using the old API (say they have older scripts).\n\nThe nargs value allows this positional argument to be optional. Hence, the test on L66 is checking to see if they have provided the argument (legacy mode), for which we take the ID (which may be a name provided and it gets converted to an ID). Otherwise, we store None, to tell the server we are using the (normal) new mechanism.\n\nThe default is that the user does not specify the subnet, which is why it is marked as deprecated in the help text, to indicate that the default is that they do not provide this value.","commit_id":"c0ee9bf3a85220a72dfc7fcebfb809cfcee212ae"}],"neutronclient/tests/functional/adv-svcs/test_readonly_neutron_vpn.py":[{"author":{"_account_id":14064,"name":"Kahou Lei","email":"kahou82@gmail.com","username":"kahou82"},"change_message_id":"515f1dfab78281c301ae0fe02dfd4965f9dc5821","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7a2fa921_70aa771b","updated":"2015-10-06 06:33:08.000000000","message":"Are we missing unit test here?","commit_id":"eba49e82afc59fc26539989574965b85df913510"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"7a0aeef0b95b79e409aef95ec50d22484c1c019c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7a2fa921_776343b2","in_reply_to":"7a2fa921_70aa771b","updated":"2015-10-06 11:10:42.000000000","message":"No. The tests only do readonly tests, which are the \"list\" variety.\n\nThe list output for the IPSec connection CLI is modified with the change set to remove peer-cidr, which will be empty, and to remove route mode, which is hard coded (and doesn\u0027t provide any value).","commit_id":"eba49e82afc59fc26539989574965b85df913510"}]}