)]}'
{"tempest/common/ssh.py":[{"author":{"_account_id":13167,"name":"Yogesh","email":"ykshirsa@netapp.com","username":"yogi1"},"change_message_id":"1898c7700d5c4c683ab292a8103a0e5dd855aeee","unresolved":false,"context_lines":[{"line_number":29,"context_line":"try:"},{"line_number":30,"context_line":"    import SocketServer"},{"line_number":31,"context_line":"except ImportError:"},{"line_number":32,"context_line":"    import socketserver as SocketServer"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"with warnings.catch_warnings():"},{"line_number":35,"context_line":"    warnings.simplefilter(\"ignore\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_5c4e9697","line":32,"updated":"2014-11-12 18:10:11.000000000","message":"Not very convinced with this try block.","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def __init__(self, host, username, password\u003dNone, timeout\u003d300, pkey\u003dNone,"},{"line_number":45,"context_line":"                 channel_timeout\u003d10, look_for_keys\u003dFalse, key_filename\u003dNone,"},{"line_number":46,"context_line":"                 gws\u003dNone):"},{"line_number":47,"context_line":"        \"\"\""},{"line_number":48,"context_line":"        Added this parameter for creating ssh tunnel, it is a list"},{"line_number":49,"context_line":"        of dictionaries describing each \"hop\" inside the tunnel."}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_276433e4","line":46,"updated":"2014-10-03 11:15:25.000000000","message":"I would put gws\u003d[]\nIt is easier to understand that you expect a list and code like:\n\n  if gws:\n     foo\n\nwill still work","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":62,"context_line":"            self.GWs \u003d gws"},{"line_number":63,"context_line":"            self.tunnels \u003d []"},{"line_number":64,"context_line":"            self.ssh_gw \u003d None"},{"line_number":65,"context_line":"        \"\"\""},{"line_number":66,"context_line":"        self.host \u003d host"},{"line_number":67,"context_line":"        self.username \u003d username"},{"line_number":68,"context_line":"        self.password \u003d password"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_0769efbd","line":65,"updated":"2014-10-03 11:15:25.000000000","message":"Docstrings are for defining the function and for creating the documentation and you are using it to define a change. As a reviewer, I think this comment is so useful to help me understand the change, but should not be inserted in the code. A comment in gerrit like this one just after the patch is pushed is the best option, IMHO.","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":13167,"name":"Yogesh","email":"ykshirsa@netapp.com","username":"yogi1"},"change_message_id":"1898c7700d5c4c683ab292a8103a0e5dd855aeee","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            \"key_filename\": self.key_filename"},{"line_number":81,"context_line":"        }"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # GW variables"},{"line_number":84,"context_line":"        self.GWs \u003d gws"},{"line_number":85,"context_line":"        self.tunnels \u003d []"},{"line_number":86,"context_line":"        self.ssh_gw \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_7f0a6422","line":83,"updated":"2014-11-12 18:10:11.000000000","message":"What is GW variable. can you make little descriptive?","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        }"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # GW variables"},{"line_number":84,"context_line":"        self.GWs \u003d gws"},{"line_number":85,"context_line":"        self.tunnels \u003d []"},{"line_number":86,"context_line":"        self.ssh_gw \u003d None"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_2792739f","line":84,"updated":"2014-10-03 11:15:25.000000000","message":"never capitalise instance attributes","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    def _build_tunnel(self):"},{"line_number":134,"context_line":"        \"\"\""},{"line_number":135,"context_line":"         Builds a ssh inception tunneling with"},{"line_number":136,"context_line":"         through GW added tot he list of GWs"},{"line_number":137,"context_line":"        \"\"\""},{"line_number":138,"context_line":"        ssh \u003d paramiko.SSHClient()"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_a73bc39a","line":135,"updated":"2014-10-03 11:15:25.000000000","message":"kudos for \u0027inception\u0027 description :)","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    def _build_tunnel(self):"},{"line_number":134,"context_line":"        \"\"\""},{"line_number":135,"context_line":"         Builds a ssh inception tunneling with"},{"line_number":136,"context_line":"         through GW added tot he list of GWs"},{"line_number":137,"context_line":"        \"\"\""},{"line_number":138,"context_line":"        ssh \u003d paramiko.SSHClient()"},{"line_number":139,"context_line":"        ssh.set_missing_host_key_policy("}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_074c8f41","line":136,"updated":"2014-10-03 11:15:25.000000000","message":"\"with through\" it sounds quite strange.\n\"to the\" list","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        for dest in self.GWs[1:]:"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"                dest[\"pkey\"] \u003d self._fix_pkey(dest[\"pkey\"])"},{"line_number":152,"context_line":"                ssh \u003d self._do_connect(dest)"},{"line_number":153,"context_line":"                self.tunnels.append(ssh)"},{"line_number":154,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_0721ef4c","line":151,"updated":"2014-10-03 11:15:25.000000000","message":"This indentation! http://cdn.memegenerator.net/instances/500x/45742643.jpg :)","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":13167,"name":"Yogesh","email":"ykshirsa@netapp.com","username":"yogi1"},"change_message_id":"1898c7700d5c4c683ab292a8103a0e5dd855aeee","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def _do_connect(self, dest, tunnel\u003dTrue):"},{"line_number":159,"context_line":"        \"\"\""},{"line_number":160,"context_line":"        Function the wraps the different \"connects\" that could appear"},{"line_number":161,"context_line":"        :param dest: dictionary with the details of the destination machine"},{"line_number":162,"context_line":"        :param tunnel: if its a tunneled connection"},{"line_number":163,"context_line":"        :return: returns the ssh paramiko client created"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_7f5984fd","line":160,"updated":"2014-11-12 18:10:11.000000000","message":"Sentence \"Function the wraps the different\" not making sense.","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                     dest[\"username\"], dest[\"ip\"])"},{"line_number":191,"context_line":"        except (socket.error,"},{"line_number":192,"context_line":"                paramiko.SSHException):"},{"line_number":193,"context_line":"                    raise"},{"line_number":194,"context_line":"        return ssh"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def _is_timed_out(self, start_time, timeout\u003d0):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_2778b32a","line":193,"updated":"2014-10-03 11:15:25.000000000","message":"If you are going to raise the same without logging or try something to fix it... Why the try/except block?","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":13167,"name":"Yogesh","email":"ykshirsa@netapp.com","username":"yogi1"},"change_message_id":"1898c7700d5c4c683ab292a8103a0e5dd855aeee","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        :param start_time: time when the process starts"},{"line_number":199,"context_line":"        :param timeout: optional value, used if we want"},{"line_number":200,"context_line":"        to use a different timeout"},{"line_number":201,"context_line":"        rather than the \"class\" timeout,"},{"line_number":202,"context_line":"        for instance giving a timeout to a particular command"},{"line_number":203,"context_line":"        :return: True if the timeout is surpassed"},{"line_number":204,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_7fac24a1","line":201,"updated":"2014-11-12 18:10:11.000000000","message":"can be continued on the same line above.","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                return port"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    @staticmethod"},{"line_number":228,"context_line":"    def _fix_pkey(pkey):"},{"line_number":229,"context_line":"        if isinstance(pkey, six.string_types):"},{"line_number":230,"context_line":"            pkey \u003d paramiko.RSAKey.from_private_key("},{"line_number":231,"context_line":"                cStringIO.StringIO(str(pkey)))"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_47ba1724","line":228,"updated":"2014-10-03 11:15:25.000000000","message":"Is there any reason why this is a static method? I\u0027ve only found self._fix_pkey calls...","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":13167,"name":"Yogesh","email":"ykshirsa@netapp.com","username":"yogi1"},"change_message_id":"1898c7700d5c4c683ab292a8103a0e5dd855aeee","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        poll \u003d select.poll()"},{"line_number":254,"context_line":"        poll.register(channel, select.POLLIN)"},{"line_number":255,"context_line":"        start_time \u003d time.time()"},{"line_number":256,"context_line":"        LOG.info(\"executing cmd: %s\" % cmd)"},{"line_number":257,"context_line":"        while True:"},{"line_number":258,"context_line":"            ready \u003d poll.poll(self.channel_timeout)"},{"line_number":259,"context_line":"            if not any(ready) or cmd_timeout is not 0:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_5f1688a8","line":256,"updated":"2014-11-12 18:10:11.000000000","message":"AFAIK, tempest don\u0027t need i18n, because its a testing framework and not visible to the end user.","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        poll \u003d select.poll()"},{"line_number":254,"context_line":"        poll.register(channel, select.POLLIN)"},{"line_number":255,"context_line":"        start_time \u003d time.time()"},{"line_number":256,"context_line":"        LOG.info(\"executing cmd: %s\" % cmd)"},{"line_number":257,"context_line":"        while True:"},{"line_number":258,"context_line":"            ready \u003d poll.poll(self.channel_timeout)"},{"line_number":259,"context_line":"            if not any(ready) or cmd_timeout is not 0:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_07d2cfff","line":256,"updated":"2014-10-03 11:15:25.000000000","message":"Not a tempest expert, but LOG.info messages do need to be i18n?","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"}],"tempest/common/utils/linux/remote_client.py":[{"author":{"_account_id":13167,"name":"Yogesh","email":"ykshirsa@netapp.com","username":"yogi1"},"change_message_id":"1898c7700d5c4c683ab292a8103a0e5dd855aeee","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    # NOTE(afazekas): It should always get an address instead of server"},{"line_number":27,"context_line":"    # Added a new parameter \"GW\" for allowing the use of multi host tunneling"},{"line_number":28,"context_line":"    def __init__(self, server, username, password\u003dNone, pkey\u003dNone, gws\u003dNone):"},{"line_number":29,"context_line":"        ssh_timeout \u003d CONF.compute.ssh_timeout"},{"line_number":30,"context_line":"        network \u003d CONF.compute.network_for_ssh"},{"line_number":31,"context_line":"        ip_version \u003d CONF.compute.ip_version_for_ssh"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_3f61bc39","line":28,"updated":"2014-11-12 18:10:11.000000000","message":"I would use more descriptive variable than just gws","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    # NOTE(afazekas): It should always get an address instead of server"},{"line_number":27,"context_line":"    # Added a new parameter \"GW\" for allowing the use of multi host tunneling"},{"line_number":28,"context_line":"    def __init__(self, server, username, password\u003dNone, pkey\u003dNone, gws\u003dNone):"},{"line_number":29,"context_line":"        ssh_timeout \u003d CONF.compute.ssh_timeout"},{"line_number":30,"context_line":"        network \u003d CONF.compute.network_for_ssh"},{"line_number":31,"context_line":"        ip_version \u003d CONF.compute.ip_version_for_ssh"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_c7de073f","line":28,"updated":"2014-10-03 11:15:25.000000000","message":"Same as before. I would put: \n\ngws\u003d[]","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"},{"author":{"_account_id":13167,"name":"Yogesh","email":"ykshirsa@netapp.com","username":"yogi1"},"change_message_id":"1898c7700d5c4c683ab292a8103a0e5dd855aeee","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def validate_authentication(self):"},{"line_number":63,"context_line":"        \"\"\"Validate ssh connection and authentication"},{"line_number":64,"context_line":"           This method raises an Exception when the validation fails."},{"line_number":65,"context_line":"        \"\"\""},{"line_number":66,"context_line":"        self.ssh_client.test_connection_auth()"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_9fa3904e","line":64,"updated":"2014-11-12 18:10:11.000000000","message":"Do you mean the method test_connection_auth() will raise the exception or you missed to raise it?","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"}],"tempest/tests/test_ssh.py":[{"author":{"_account_id":7505,"name":"Jaume Devesa","email":"devvesa@gmail.com","username":"devesa"},"change_message_id":"a2dee4cf97f8964bee83e30bbaaa2908c31745a1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        self.assertEqual(2, aa_mock.call_count)"},{"line_number":68,"context_line":"        # client_mock.set_missing_host_key_policy.assert_called_once_with("},{"line_number":69,"context_line":"        #    mock.sentinel.aa)"},{"line_number":70,"context_line":"        # same as before."},{"line_number":71,"context_line":"        kall \u003d client_mock.set_missing_host_key_policy.call_args"},{"line_number":72,"context_line":"        args, kwargs \u003d kall"},{"line_number":73,"context_line":"        self.assertEqual(args[0], mock.sentinel.aa)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_67d71bee","line":70,"updated":"2014-10-03 11:15:25.000000000","message":"Remove commented lines","commit_id":"dba893d8c0a84f675b6e892bb50c0820be729d7d"}]}