)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16788,"name":"Ramanjaneya Reddy Palleti","email":"ramanjaneya.palleti@huawei.com","username":"Ramanjaneya"},"change_message_id":"9aa0f76504c1f0ea6ec5a9bb75e794e307e2b776","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Rawlin Peters \u003crawlin.peters@hpe.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-10-26 17:30:34 -0600"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP: DO NOT MERGE: Batch ARP updates"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I8700f309c34425cd8938571e626b187ee713eab1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a29b11f_f64adfab","line":7,"updated":"2015-10-27 03:51:45.000000000","message":"Write unit test cases as well","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"}],"neutron/agent/l3/dvr_edge_ha_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"568a7df32eae6dcbd86ccc21dbd61f6151f50712","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        batch_ip_cmd \u003d ip_lib.BatchIpCommand(self.ns_name)"},{"line_number":50,"context_line":"        for subnet in port[\u0027subnets\u0027]:"},{"line_number":51,"context_line":"            self._set_subnet_arp_info(subnet[\u0027id\u0027], batch_ip_cmd)"},{"line_number":52,"context_line":"        batch_ip_cmd.execute()"},{"line_number":53,"context_line":"        self._snat_redirect_add_from_port(port)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not self.get_ex_gw_port() or not self._is_this_snat_host():"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_3f7e15e5","line":52,"updated":"2015-10-27 20:49:23.000000000","message":"It\u0027d be cool if BatchIpCommand could be used like this to obviate the need to remember that there must be a call to execute:\n\n  with ip_lib.BatchIpCommand(self.ns_name) is batch_ip_cmd:\n      for subnet in ...\n\nI think it is poor pattern to have lines of code like L49 and L52 that are intrinsically couple together in purpose but I separated by multiple lines of code.","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"2cfe7794ca8093f348ae8727f14cffbc4d99d04e","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        batch_ip_cmd \u003d ip_lib.BatchIpCommand(self.ns_name)"},{"line_number":50,"context_line":"        for subnet in port[\u0027subnets\u0027]:"},{"line_number":51,"context_line":"            self._set_subnet_arp_info(subnet[\u0027id\u0027], batch_ip_cmd)"},{"line_number":52,"context_line":"        batch_ip_cmd.execute()"},{"line_number":53,"context_line":"        self._snat_redirect_add_from_port(port)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not self.get_ex_gw_port() or not self._is_this_snat_host():"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_1a0567c1","line":52,"in_reply_to":"1a26ad4f_3f7e15e5","updated":"2015-10-27 21:53:57.000000000","message":"Yeah I agree, I vaguely thought of doing something like that, and implementing it as a context manager seems like the best design. I\u0027ll try that out and see where it goes.","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"}],"neutron/agent/linux/ip_lib.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"568a7df32eae6dcbd86ccc21dbd61f6151f50712","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        return self.log_fail_as_error"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"class BatchIpCommand(SubProcessBase):"},{"line_number":107,"context_line":"    def __init__(self, namespace\u003dNone):"},{"line_number":108,"context_line":"        super(BatchIpCommand, self).__init__(namespace\u003dnamespace)"},{"line_number":109,"context_line":"        self._v4_cmd_list \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_dfe7e1aa","line":106,"updated":"2015-10-27 20:49:23.000000000","message":"I actually played around with doing something like this a couple of years ago.  My memory is fuzzy, I don\u0027t remember exactly why I abandoned the idea.  I think I was going after batching more heterogenous sets of commands together and doing more complicated batching.  But, what you seem to be doing here is batching homogenous sets of commands which is probably a lot simpler.  So, it probably has a better chance of working.\n\nThis kind of addition should be very well covered with unit tests.","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"568a7df32eae6dcbd86ccc21dbd61f6151f50712","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        self._v4_cmd_list \u003d []"},{"line_number":110,"context_line":"        self._v6_cmd_list \u003d []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def add_v4_cmd(self, cmd):"},{"line_number":113,"context_line":"        self._v4_cmd_list.append(cmd)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def add_v6_cmd(self, cmd):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_ffbadd6d","line":112,"updated":"2015-10-27 20:49:23.000000000","message":"There is no good reason you can\u0027t combine this method with the next one by adding an ip_version parameter.  You\u0027d have to do something a little bit different with self._v4_cmd_list and self._v6_cmd_list but that shouldn\u0027t be hard:\n\n  self._cmd_list_by_ip_version \u003d {4: [], 6: []}","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"2cfe7794ca8093f348ae8727f14cffbc4d99d04e","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        self._v4_cmd_list \u003d []"},{"line_number":110,"context_line":"        self._v6_cmd_list \u003d []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def add_v4_cmd(self, cmd):"},{"line_number":113,"context_line":"        self._v4_cmd_list.append(cmd)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def add_v6_cmd(self, cmd):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_7a9c9b69","line":112,"in_reply_to":"1a26ad4f_ffbadd6d","updated":"2015-10-27 21:53:57.000000000","message":"I was actually in the process of eliminating this whole ipv4 vs ipv6 stuff from BatchIpCommand, because specifying \"-4\" or \"-6\" for the ip command doesn\u0027t seem to bring much benefit when batching here. It makes sense to specify the ip version for single commands, but splitting the v4 and v6 commands into separate batches and running each batch separately will lead to more code complexity and less overall performance IMO.","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"568a7df32eae6dcbd86ccc21dbd61f6151f50712","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        self._run_v4()"},{"line_number":120,"context_line":"        self._run_v6()"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    def _run_v4(self):"},{"line_number":123,"context_line":"        if not self._v4_cmd_list:"},{"line_number":124,"context_line":"            return"},{"line_number":125,"context_line":"        v4_cmd_input \u003d \"\\n\".join(self._v4_cmd_list)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_1fbc796b","line":122,"updated":"2015-10-27 20:49:23.000000000","message":"ditto (L112)","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"568a7df32eae6dcbd86ccc21dbd61f6151f50712","unresolved":false,"context_lines":[{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def add_batch_cmd(self, ip_version, cmd):"},{"line_number":355,"context_line":"        if self._parent.batch_ip_cmd:"},{"line_number":356,"context_line":"            if ip_version \u003d\u003d 4:"},{"line_number":357,"context_line":"                self._parent.batch_ip_cmd.add_v4_cmd(cmd)"},{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                self._parent.batch_ip_cmd.add_v6_cmd(cmd)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_df6cc10a","line":356,"updated":"2015-10-27 20:49:23.000000000","message":"Use a constant rather than a hard-coded 4.","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"568a7df32eae6dcbd86ccc21dbd61f6151f50712","unresolved":false,"context_lines":[{"line_number":863,"context_line":"                \u0027nud\u0027, \u0027permanent\u0027,"},{"line_number":864,"context_line":"                \u0027dev\u0027, self.name)"},{"line_number":865,"context_line":"        if self.is_batched():"},{"line_number":866,"context_line":"            self.add_batch_cmd(ip_version, \u0027 \u0027.join((self.COMMAND,) + args))"},{"line_number":867,"context_line":"        else:"},{"line_number":868,"context_line":"            self._as_root([ip_version], args)"},{"line_number":869,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_df3a81e7","line":866,"updated":"2015-10-27 20:49:23.000000000","message":"I would if this \"if is_batched\" stuff could be done at a lower level like in self._as_root or lower.\n\nAlso, I\u0027m not keen how this method is responsible for joining the command with the args.  That seems like a detail that should be taken care of at a lower level.","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"2cfe7794ca8093f348ae8727f14cffbc4d99d04e","unresolved":false,"context_lines":[{"line_number":863,"context_line":"                \u0027nud\u0027, \u0027permanent\u0027,"},{"line_number":864,"context_line":"                \u0027dev\u0027, self.name)"},{"line_number":865,"context_line":"        if self.is_batched():"},{"line_number":866,"context_line":"            self.add_batch_cmd(ip_version, \u0027 \u0027.join((self.COMMAND,) + args))"},{"line_number":867,"context_line":"        else:"},{"line_number":868,"context_line":"            self._as_root([ip_version], args)"},{"line_number":869,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_ba79d331","line":866,"in_reply_to":"1a26ad4f_df3a81e7","updated":"2015-10-27 21:53:57.000000000","message":"I agree with you on both of these points.","commit_id":"b93104cf1ef1b3f9015dec7a049b8677093e859a"},{"author":{"_account_id":16788,"name":"Ramanjaneya Reddy Palleti","email":"ramanjaneya.palleti@huawei.com","username":"Ramanjaneya"},"change_message_id":"803d9b729cf31c78bdd988a417686d4722cd7cd2","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            self._as_root(options\u003d[\u0027force\u0027, \u0027b\u0027, \u0027\u0027], command\u003d\u0027\u0027,"},{"line_number":130,"context_line":"                          args\u003d(), process_input\u003dcmd_input)"},{"line_number":131,"context_line":"        except RuntimeError as rte:"},{"line_number":132,"context_line":"            with excutils.save_and_reraise_exception() as ctx:"},{"line_number":133,"context_line":"                for line in rte.message.split(\u0027\\n\u0027):"},{"line_number":134,"context_line":"                    if \u0027Command failed\u0027 in line:  # TODO(Rawlin): regex here?"},{"line_number":135,"context_line":"                        index \u003d int(line.split(\u0027:\u0027)[-1]) - 1"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a26ad4f_bd58e357","line":132,"updated":"2015-10-28 06:46:56.000000000","message":"Please fix pep8 error, ctx never used","commit_id":"493d9fcd0dc28424cc9af649afa42809e5e64098"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a5373bb4d5ce421324f386933f05fa60d7c7ab7e","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    def __init__(self, name, namespace\u003dNone, batch_ip_cmd\u003dNone):"},{"line_number":275,"context_line":"        super(IPDevice, self).__init__(namespace\u003dnamespace)"},{"line_number":276,"context_line":"        self.name \u003d name"},{"line_number":277,"context_line":"        self.batch_ip_cmd \u003d batch_ip_cmd"},{"line_number":278,"context_line":"        self.link \u003d IpLinkCommand(self)"},{"line_number":279,"context_line":"        self.addr \u003d IpAddrCommand(self)"},{"line_number":280,"context_line":"        self.route \u003d IpRouteCommand(self)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a26ad4f_bfd36903","line":277,"updated":"2015-11-02 23:08:42.000000000","message":"I think you could tweak this slightly to use the same logic for checking device existence, instead of relying on the check_devices call on L112.  See this and follow-on comments below.  Although I am still half-asleep...\n\nself._exists \u003d False","commit_id":"1fb444792ac1d97488fc8f3bb7a7d586ece951a5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a5373bb4d5ce421324f386933f05fa60d7c7ab7e","unresolved":false,"context_lines":[{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def exists(self):"},{"line_number":291,"context_line":"        \"\"\"Return True if the device exists in the namespace.\"\"\""},{"line_number":292,"context_line":"        # we must save and restore this before returning"},{"line_number":293,"context_line":"        if self.batch_ip_cmd:"},{"line_number":294,"context_line":"            return self.name in self.batch_ip_cmd._existing_devices"},{"line_number":295,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a26ad4f_1fe05df9","line":292,"updated":"2015-11-02 23:08:42.000000000","message":"nit: this comment belongs to orig_log_fail_as_error so should stay just above it.","commit_id":"1fb444792ac1d97488fc8f3bb7a7d586ece951a5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a5373bb4d5ce421324f386933f05fa60d7c7ab7e","unresolved":false,"context_lines":[{"line_number":291,"context_line":"        \"\"\"Return True if the device exists in the namespace.\"\"\""},{"line_number":292,"context_line":"        # we must save and restore this before returning"},{"line_number":293,"context_line":"        if self.batch_ip_cmd:"},{"line_number":294,"context_line":"            return self.name in self.batch_ip_cmd._existing_devices"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        orig_log_fail_as_error \u003d self.get_log_fail_as_error()"},{"line_number":297,"context_line":"        self.set_log_fail_as_error(False)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a26ad4f_7fd95121","line":294,"updated":"2015-11-02 23:08:42.000000000","message":"if self._exists:\n    return True","commit_id":"1fb444792ac1d97488fc8f3bb7a7d586ece951a5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a5373bb4d5ce421324f386933f05fa60d7c7ab7e","unresolved":false,"context_lines":[{"line_number":296,"context_line":"        orig_log_fail_as_error \u003d self.get_log_fail_as_error()"},{"line_number":297,"context_line":"        self.set_log_fail_as_error(False)"},{"line_number":298,"context_line":"        try:"},{"line_number":299,"context_line":"            return bool(self.link.address)"},{"line_number":300,"context_line":"        except RuntimeError:"},{"line_number":301,"context_line":"            return False"},{"line_number":302,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a26ad4f_1f013d6a","line":299,"updated":"2015-11-02 23:08:42.000000000","message":"self._exists \u003d bool(self.link.address)\nreturn self._exists","commit_id":"1fb444792ac1d97488fc8f3bb7a7d586ece951a5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"b8918e29be0e166c27bd2bbb41b02f65cebaff82","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    def exists(self):"},{"line_number":292,"context_line":"        \"\"\"Return True if the device exists in the namespace.\"\"\""},{"line_number":293,"context_line":"        if self.batch_ip_cmd:"},{"line_number":294,"context_line":"            return self.name in self.batch_ip_cmd._existing_devices"},{"line_number":295,"context_line":"        if self._exists:"},{"line_number":296,"context_line":"            return True"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa80f949_14864ab9","line":294,"updated":"2015-11-05 15:58:51.000000000","message":"Maybe I confused you (or myself), but I don\u0027t think batch_ip_cmd is needed any more.  I was trying to make this more generic by having the class keep track of existence instead of having to pass batch_ip_cmd to it.  If that doesn\u0027t work then _exists can go away.","commit_id":"bb2472c293aa2f6dbf5fd3a0a0523ea3bebe459e"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"27dccbdbbc7b8730c19b5d1c3a2227a08d202de1","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    def exists(self):"},{"line_number":292,"context_line":"        \"\"\"Return True if the device exists in the namespace.\"\"\""},{"line_number":293,"context_line":"        if self.batch_ip_cmd:"},{"line_number":294,"context_line":"            return self.name in self.batch_ip_cmd._existing_devices"},{"line_number":295,"context_line":"        if self._exists:"},{"line_number":296,"context_line":"            return True"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa80f949_23a6ae10","line":294,"in_reply_to":"fa80f949_14864ab9","updated":"2015-11-05 17:17:11.000000000","message":"Ah, I wasn\u0027t sure if that was your intention or not. Just adding the _exists variable won\u0027t help with batching ARP updates because a new IPDevice object is created for every single ARP update. The _exists variable would help in cases where the lifetime of the IPDevice object spans multiple checks for existence but doesn\u0027t have a batch_ip_cmd. I\u0027m not sure if it\u0027s being used elsewhere in the code in that manner, but it would provide some benefit if it is.","commit_id":"bb2472c293aa2f6dbf5fd3a0a0523ea3bebe459e"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"e64b91cc56259692e8b3ca57ea79afd4bf7783dd","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                for line in rte.message.split(\u0027\\n\u0027):"},{"line_number":136,"context_line":"                    if \u0027Command failed\u0027 in line:  # TODO(Rawlin): regex here?"},{"line_number":137,"context_line":"                        index \u003d int(line.split(\u0027:\u0027)[-1]) - 1"},{"line_number":138,"context_line":"                        self._failed_commands.append(self._cmd_list[index])"},{"line_number":139,"context_line":"                self._cmd_list \u003d []"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_3303cbba","line":138,"updated":"2015-11-07 10:23:44.000000000","message":"well, am I missing something? what will we go with _failed_commands ?","commit_id":"c11dc85c44dc83cf1f0effab2346bf40de5b87a4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"b68d27658e21cf5fe926ad681c8fb82ea7e9adf7","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                for line in rte.message.split(\u0027\\n\u0027):"},{"line_number":136,"context_line":"                    if \u0027Command failed\u0027 in line:  # TODO(Rawlin): regex here?"},{"line_number":137,"context_line":"                        index \u003d int(line.split(\u0027:\u0027)[-1]) - 1"},{"line_number":138,"context_line":"                        self._failed_commands.append(self._cmd_list[index])"},{"line_number":139,"context_line":"                self._cmd_list \u003d []"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_b2d77ebe","line":138,"in_reply_to":"da85f559_3303cbba","updated":"2015-11-09 18:18:03.000000000","message":"Hi ZongKai, thanks for the review! The _failed_commands list is just an idea I had for cases where we might want to catch this exception and possibly retry the failed commands. This change doesn\u0027t do anything with the failed commands, but I figured it should be included in the BatchIpCommand class just in case someone needs it in the future.","commit_id":"c11dc85c44dc83cf1f0effab2346bf40de5b87a4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"e64b91cc56259692e8b3ca57ea79afd4bf7783dd","unresolved":false,"context_lines":[{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _as_root(self, options, args, use_root_namespace\u003dFalse):"},{"line_number":352,"context_line":"        if self.is_batched():"},{"line_number":353,"context_line":"            if options:"},{"line_number":354,"context_line":"                LOG.warning(_LW(\"Options are ignored when using a batched ip \""},{"line_number":355,"context_line":"                                \"command: %s\"), options)"},{"line_number":356,"context_line":"            self.add_batch_cmd(args)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_93dab777","line":353,"updated":"2015-11-07 10:23:44.000000000","message":"I may get wrong. But I think most options used for _as_root are []/[ip_version], so I think maybe we can:\n\n1) for [] and [ip-version-4] case, use _cmd_list, and for [ip-version-6] case use _v6_cmd_list, to prevent special usage not being misused.\n\n2) if 1) can be done, we may don\u0027t need waring here.","commit_id":"c11dc85c44dc83cf1f0effab2346bf40de5b87a4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"b68d27658e21cf5fe926ad681c8fb82ea7e9adf7","unresolved":false,"context_lines":[{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _as_root(self, options, args, use_root_namespace\u003dFalse):"},{"line_number":352,"context_line":"        if self.is_batched():"},{"line_number":353,"context_line":"            if options:"},{"line_number":354,"context_line":"                LOG.warning(_LW(\"Options are ignored when using a batched ip \""},{"line_number":355,"context_line":"                                \"command: %s\"), options)"},{"line_number":356,"context_line":"            self.add_batch_cmd(args)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_5d2183fa","line":353,"in_reply_to":"da85f559_93dab777","updated":"2015-11-09 18:18:03.000000000","message":"I believe you are right in that most options are just the ip version. I think another option which is used is \"-o\", and there may be others. If you take a look at PS1, using separate command lists for v4 and v6 and executing them separately was the approach I started with. However, I ended up changing that because it didn\u0027t seem to add much benefit for the complexity that it added.\n\nAnd in hindsight, I think logging this as a warning is probably too extreme here, it should probably just be debug.","commit_id":"c11dc85c44dc83cf1f0effab2346bf40de5b87a4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"32f35ce4e167491acbac4fa9d6ae5bfcffbd79b2","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        return self"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def __exit__(self, exc_type, exc_val, exc_tb):"},{"line_number":118,"context_line":"        if not exc_type and not exc_val and not exc_tb:"},{"line_number":119,"context_line":"            self.execute()"},{"line_number":120,"context_line":"        else:"},{"line_number":121,"context_line":"            pass  # TODO(Rawlin): do something with the exception?"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_e1a98d7c","line":118,"updated":"2015-11-17 23:09:02.000000000","message":"nit: not(...)","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"6ebe1f97968eb51c46105b4fa2ddb63e4f78964c","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        return self"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def __exit__(self, exc_type, exc_val, exc_tb):"},{"line_number":118,"context_line":"        if not exc_type and not exc_val and not exc_tb:"},{"line_number":119,"context_line":"            self.execute()"},{"line_number":120,"context_line":"        else:"},{"line_number":121,"context_line":"            pass  # TODO(Rawlin): do something with the exception?"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_c3069b85","line":118,"in_reply_to":"ba8a016a_e1a98d7c","updated":"2015-11-18 21:44:44.000000000","message":"Done","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"32f35ce4e167491acbac4fa9d6ae5bfcffbd79b2","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                for line in str(rte).split(\u0027\\n\u0027):"},{"line_number":136,"context_line":"                    if \u0027Command failed\u0027 in line:  # TODO(Rawlin): regex here?"},{"line_number":137,"context_line":"                        index \u003d int(line.split(\u0027:\u0027)[-1]) - 1"},{"line_number":138,"context_line":"                        self._failed_commands.append(self._cmd_list[index])"},{"line_number":139,"context_line":"        finally:"},{"line_number":140,"context_line":"            self._cmd_list \u003d []"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_449ae764","line":138,"updated":"2015-11-17 23:09:02.000000000","message":"So only the tests ever check self._failed_commands, and noone else ever re-runs them since it could just keep failing.  I\u0027m just wondering what good it is keeping them since the error should already be logged.","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"6ebe1f97968eb51c46105b4fa2ddb63e4f78964c","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                for line in str(rte).split(\u0027\\n\u0027):"},{"line_number":136,"context_line":"                    if \u0027Command failed\u0027 in line:  # TODO(Rawlin): regex here?"},{"line_number":137,"context_line":"                        index \u003d int(line.split(\u0027:\u0027)[-1]) - 1"},{"line_number":138,"context_line":"                        self._failed_commands.append(self._cmd_list[index])"},{"line_number":139,"context_line":"        finally:"},{"line_number":140,"context_line":"            self._cmd_list \u003d []"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_a85d767a","line":138,"in_reply_to":"ba8a016a_449ae764","updated":"2015-11-18 21:44:44.000000000","message":"Yeah I kind of thought it would be a good thing to have in the back of our pockets if the case ever arose where we\u0027d want to do something else with the failed commands. But I guess if that case ever arose, this code could be added back in if necessary.","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"32f35ce4e167491acbac4fa9d6ae5bfcffbd79b2","unresolved":false,"context_lines":[{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def is_batched(self):"},{"line_number":366,"context_line":"        try:"},{"line_number":367,"context_line":"            res \u003d bool(self._parent.batch_ip_cmd)"},{"line_number":368,"context_line":"        except AttributeError:"},{"line_number":369,"context_line":"            res \u003d False"},{"line_number":370,"context_line":"        return res"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_81752199","line":367,"updated":"2015-11-17 23:09:02.000000000","message":"nit: Since there\u0027s not finally: here this can just be \u0027return bool(...)\u0027 and L369 \u0027return False\u0027","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"6ebe1f97968eb51c46105b4fa2ddb63e4f78964c","unresolved":false,"context_lines":[{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def is_batched(self):"},{"line_number":366,"context_line":"        try:"},{"line_number":367,"context_line":"            res \u003d bool(self._parent.batch_ip_cmd)"},{"line_number":368,"context_line":"        except AttributeError:"},{"line_number":369,"context_line":"            res \u003d False"},{"line_number":370,"context_line":"        return res"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_a301176c","line":367,"in_reply_to":"ba8a016a_81752199","updated":"2015-11-18 21:44:44.000000000","message":"Done","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"}],"neutron/tests/unit/agent/l3/test_agent.py":[{"author":{"_account_id":16788,"name":"Ramanjaneya Reddy Palleti","email":"ramanjaneya.palleti@huawei.com","username":"Ramanjaneya"},"change_message_id":"eea46339cd74bed68ef938ee85c3b0856b84516a","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            self.assertEqual(2, ri._internal_network_added.call_count)"},{"line_number":352,"context_line":"            # ri._set_subnet_arp_info.assert_called_once_with(subnet_id)"},{"line_number":353,"context_line":"            self.assertEqual(1, ri._set_subnet_arp_info.call_count)"},{"line_number":354,"context_line":"            self.assertEqual(subnet_id, ri._set_subnet_arp_info.call_args[0][0])"},{"line_number":355,"context_line":"            self.assertIsInstance(ri._set_subnet_arp_info.call_args[0][1],"},{"line_number":356,"context_line":"                                  ip_lib.BatchIpCommand)"},{"line_number":357,"context_line":"            ri._internal_network_added.assert_called_with("}],"source_content_type":"text/x-python","patch_set":3,"id":"1a26ad4f_3a835058","line":354,"updated":"2015-10-29 09:10:17.000000000","message":"more than 80 charcters, pep8 fail","commit_id":"3af16e6166c629386de3eb0252d548ef809f5f57"}],"neutron/tests/unit/agent/linux/test_ip_lib.py":[{"author":{"_account_id":16788,"name":"Ramanjaneya Reddy Palleti","email":"ramanjaneya.palleti@huawei.com","username":"Ramanjaneya"},"change_message_id":"cac4773d296ce4467a4ccaab19377e187fc1b271","unresolved":false,"context_lines":[{"line_number":569,"context_line":""},{"line_number":570,"context_line":"    def test_is_batched_has_batch_ip_cmd(self):"},{"line_number":571,"context_line":"        self.ip.batch_ip_cmd \u003d mock.ANY"},{"line_number":572,"context_line":"        self.assertEqual(True, self.ip_cmd.is_batched())"},{"line_number":573,"context_line":""},{"line_number":574,"context_line":"    def test_add_batch_cmd(self):"},{"line_number":575,"context_line":"        self.ip.batch_ip_cmd \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_6377063b","line":572,"updated":"2015-11-09 18:48:06.000000000","message":"pep8: \nUse assertTrue instead ofassertEqual","commit_id":"c11dc85c44dc83cf1f0effab2346bf40de5b87a4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a3818c4484fa93db86d3bcc2f79e1cac025e9c18","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":266,"context_line":"        self.assertEqual(1, len(self.batch_ip_cmd._cmd_list))"},{"line_number":267,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":268,"context_line":"        self.assertEqual(2, len(self.batch_ip_cmd._cmd_list))"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @mock.patch.object(ip_lib.BatchIpCommand, \u0027_as_root\u0027)"},{"line_number":271,"context_line":"    def test_execute_remove_cmd_list_with_exception(self, mock_as_root):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da85f559_83a54a48","line":268,"updated":"2015-11-14 08:17:16.000000000","message":"nit: (I tried find some good reasons that we should change _cmd_list from list to set, but I didn\u0027t find any. It should be a list, so ) it seems no need to test and assert twice.","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"d6f2548f566bae3402782913fa908b29d2b6cfa7","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":266,"context_line":"        self.assertEqual(1, len(self.batch_ip_cmd._cmd_list))"},{"line_number":267,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":268,"context_line":"        self.assertEqual(2, len(self.batch_ip_cmd._cmd_list))"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @mock.patch.object(ip_lib.BatchIpCommand, \u0027_as_root\u0027)"},{"line_number":271,"context_line":"    def test_execute_remove_cmd_list_with_exception(self, mock_as_root):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_01e894e1","line":268,"in_reply_to":"da85f559_83a54a48","updated":"2015-11-16 21:50:00.000000000","message":"Done","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a3818c4484fa93db86d3bcc2f79e1cac025e9c18","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":278,"context_line":"        except RuntimeError:"},{"line_number":279,"context_line":"            pass"},{"line_number":280,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    @mock.patch.object(ip_lib.BatchIpCommand, \u0027_as_root\u0027)"},{"line_number":283,"context_line":"    def test_execute_remove_cmd_list(self, mock_as_root):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da85f559_234a5e84","line":280,"updated":"2015-11-14 08:17:16.000000000","message":"nit: since test_execute_remove_cmd_list will assert self.batch_ip_cmd._cmd_list size will turn to 0, shall this test test something different?","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"d6f2548f566bae3402782913fa908b29d2b6cfa7","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":278,"context_line":"        except RuntimeError:"},{"line_number":279,"context_line":"            pass"},{"line_number":280,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    @mock.patch.object(ip_lib.BatchIpCommand, \u0027_as_root\u0027)"},{"line_number":283,"context_line":"    def test_execute_remove_cmd_list(self, mock_as_root):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_81a1e4ab","line":280,"in_reply_to":"da85f559_234a5e84","updated":"2015-11-16 21:50:00.000000000","message":"I\u0027ll add the assertion from this case into test_execute_with_exception and remove this case.","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a3818c4484fa93db86d3bcc2f79e1cac025e9c18","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        mock_as_root.assert_called_once_with(options\u003d[\u0027force\u0027, \u0027b\u0027, \u0027\u0027],"},{"line_number":305,"context_line":"                                             command\u003d\u0027\u0027,"},{"line_number":306,"context_line":"                                             args\u003d(),"},{"line_number":307,"context_line":"                                             process_input\u003d\u0027foo bar\\nfoo bar\u0027)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    @mock.patch.object(ip_lib.BatchIpCommand, \u0027_as_root\u0027)"},{"line_number":310,"context_line":"    def test_execute_cmd_list_empty(self, mock_as_root):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da85f559_1eff7102","line":307,"updated":"2015-11-14 08:17:16.000000000","message":"I think previous three test cases can be combined into one:\n\n1) every time execute is called, _cmd_list should be empty, so it makes sense to move assertions in test_execute_remove_cmd_list into the other two methods;\n\n2) to execute, it should just care _cmd_list is empty or not, so run one command should have nothing difference from multiple ones.","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"d6f2548f566bae3402782913fa908b29d2b6cfa7","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        mock_as_root.assert_called_once_with(options\u003d[\u0027force\u0027, \u0027b\u0027, \u0027\u0027],"},{"line_number":305,"context_line":"                                             command\u003d\u0027\u0027,"},{"line_number":306,"context_line":"                                             args\u003d(),"},{"line_number":307,"context_line":"                                             process_input\u003d\u0027foo bar\\nfoo bar\u0027)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    @mock.patch.object(ip_lib.BatchIpCommand, \u0027_as_root\u0027)"},{"line_number":310,"context_line":"    def test_execute_cmd_list_empty(self, mock_as_root):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_c1c4cc41","line":307,"in_reply_to":"da85f559_1eff7102","updated":"2015-11-16 21:50:00.000000000","message":"I think test_execute_remove_cmd_list could be removed and the assertions moved into test_execute_one_cmd and test_execute_multiple_cmd. However, the difference between running one command and running multiple is that multiple commands are joined with \u0027\\n\u0027, so I think it makes sense to test that.","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a3818c4484fa93db86d3bcc2f79e1cac025e9c18","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":321,"context_line":"        except RuntimeError:"},{"line_number":322,"context_line":"            pass"},{"line_number":323,"context_line":"        self.assertEqual([\u0027cmd1\u0027], self.batch_ip_cmd._failed_commands)"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"class TestIpWrapper(base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":10,"id":"da85f559_be0e6530","line":323,"updated":"2015-11-14 08:17:16.000000000","message":"this test case is better than test_execute_remove_cmd_list_with_exception, can we also merge test_execute_remove_cmd_list_with_exception into this one?","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"d6f2548f566bae3402782913fa908b29d2b6cfa7","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":321,"context_line":"        except RuntimeError:"},{"line_number":322,"context_line":"            pass"},{"line_number":323,"context_line":"        self.assertEqual([\u0027cmd1\u0027], self.batch_ip_cmd._failed_commands)"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"class TestIpWrapper(base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_e11790eb","line":323,"in_reply_to":"da85f559_be0e6530","updated":"2015-11-16 21:50:00.000000000","message":"Sure, I will just add the assertion from test_execute_remove_cmd_list_with_exception to this test case and remove that test case.","commit_id":"fa9082524973ee1d5277c3d3ef704d004babcf1b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"32f35ce4e167491acbac4fa9d6ae5bfcffbd79b2","unresolved":false,"context_lines":[{"line_number":281,"context_line":"    def test_execute_multiple_cmd(self, mock_as_root):"},{"line_number":282,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"},{"line_number":283,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":284,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":285,"context_line":"        self.assertEqual(2, len(self.batch_ip_cmd._cmd_list))"},{"line_number":286,"context_line":"        self.batch_ip_cmd.execute()"},{"line_number":287,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_849c4fc1","line":284,"updated":"2015-11-17 23:09:02.000000000","message":"nit: should probably make this a different string","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"6ebe1f97968eb51c46105b4fa2ddb63e4f78964c","unresolved":false,"context_lines":[{"line_number":281,"context_line":"    def test_execute_multiple_cmd(self, mock_as_root):"},{"line_number":282,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"},{"line_number":283,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":284,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027foo bar\u0027)"},{"line_number":285,"context_line":"        self.assertEqual(2, len(self.batch_ip_cmd._cmd_list))"},{"line_number":286,"context_line":"        self.batch_ip_cmd.execute()"},{"line_number":287,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_3e6bfcb8","line":284,"in_reply_to":"ba8a016a_849c4fc1","updated":"2015-11-18 21:44:44.000000000","message":"Done","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"32f35ce4e167491acbac4fa9d6ae5bfcffbd79b2","unresolved":false,"context_lines":[{"line_number":299,"context_line":"    def test_execute_with_exception(self, mock_as_root):"},{"line_number":300,"context_line":"        mock_as_root.side_effect \u003d RuntimeError(\u0027Command failed -:1\u0027)"},{"line_number":301,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027cmd1\u0027)"},{"line_number":302,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027cmd2\u0027)"},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":305,"context_line":"        except RuntimeError:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_04e93fe6","line":302,"updated":"2015-11-17 23:09:02.000000000","message":"So if there was a \u0027cmd3\u0027 here and running \u0027cmd2\u0027 threw the exception, would \u0027cmd3\u0027 get executed?  If so then you should have three side effects, something like:\n\n.side_effect \u003d [(None, RuntimeError(\u0027Command failed -:1\u0027), None)]\n\nThat way you can verify \u0027cmd3\u0027 was executed and not lost.","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"09fe75e4709623d87b11fb752881461747425f51","unresolved":false,"context_lines":[{"line_number":299,"context_line":"    def test_execute_with_exception(self, mock_as_root):"},{"line_number":300,"context_line":"        mock_as_root.side_effect \u003d RuntimeError(\u0027Command failed -:1\u0027)"},{"line_number":301,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027cmd1\u0027)"},{"line_number":302,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027cmd2\u0027)"},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":305,"context_line":"        except RuntimeError:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_dd69bebf","line":302,"in_reply_to":"ba8a016a_04e93fe6","updated":"2015-11-18 06:21:38.000000000","message":"Agree, that will make this test better.","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"6ebe1f97968eb51c46105b4fa2ddb63e4f78964c","unresolved":false,"context_lines":[{"line_number":299,"context_line":"    def test_execute_with_exception(self, mock_as_root):"},{"line_number":300,"context_line":"        mock_as_root.side_effect \u003d RuntimeError(\u0027Command failed -:1\u0027)"},{"line_number":301,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027cmd1\u0027)"},{"line_number":302,"context_line":"        self.batch_ip_cmd.add_cmd(\u0027cmd2\u0027)"},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":305,"context_line":"        except RuntimeError:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_3cab0577","line":302,"in_reply_to":"ba8a016a_04e93fe6","updated":"2015-11-18 21:44:44.000000000","message":"Because _as_root is only called once per BatchIpCommand.execute(), using that list of side effects doesn\u0027t actually work. _as_root would be called once with the batch of 3 commands and wouldn\u0027t raise an exception because the 1st side effect is None.\n\nPerhaps that would be a good functional test?","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"32f35ce4e167491acbac4fa9d6ae5bfcffbd79b2","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":305,"context_line":"        except RuntimeError:"},{"line_number":306,"context_line":"            pass"},{"line_number":307,"context_line":"        self.assertEqual([\u0027cmd1\u0027], self.batch_ip_cmd._failed_commands)"},{"line_number":308,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_c44c9772","line":306,"updated":"2015-11-17 23:09:02.000000000","message":"self.assertRaises(RuntimeError, self.batch_ip_cmd.execute())","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"6ebe1f97968eb51c46105b4fa2ddb63e4f78964c","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            self.batch_ip_cmd.execute()"},{"line_number":305,"context_line":"        except RuntimeError:"},{"line_number":306,"context_line":"            pass"},{"line_number":307,"context_line":"        self.assertEqual([\u0027cmd1\u0027], self.batch_ip_cmd._failed_commands)"},{"line_number":308,"context_line":"        self.assertEqual(0, len(self.batch_ip_cmd._cmd_list))"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba8a016a_1ed718ee","line":306,"in_reply_to":"ba8a016a_c44c9772","updated":"2015-11-18 21:44:44.000000000","message":"Done","commit_id":"20efa4042ff6fca1c50ab419111c3ede0d0f4bd4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"31860d08848561f7685283d32b093f608f888fc7","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                                             process_input\u003d\u0027foo bar\\nspam ham\u0027)"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    @mock.patch.object(ip_lib.BatchIpCommand, \u0027_as_root\u0027)"},{"line_number":294,"context_line":"    def test_execute_cmd_list_empty(self, mock_as_root):"},{"line_number":295,"context_line":"        self.batch_ip_cmd.execute()"},{"line_number":296,"context_line":"        mock_as_root.assert_not_called()"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"ba8a016a_110a9879","line":294,"updated":"2015-11-19 04:12:12.000000000","message":"nit: no need to clean _cmd_list first ?","commit_id":"1b1c8d08cd57024a605633a9cb12b8fd8b0bbc43"}]}