)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"72b08b3f86cc6c8e9d354f2820ebaa99c87673a7","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix HDS HNAS snapshot creation always succeeding"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"HDS HNAS Manila driver returns success related to job submission"},{"line_number":10,"context_line":"when creating snapshots. This patch fixes it by tracking the job"},{"line_number":11,"context_line":"status and reporting success only when the job succeeds."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Partially implements bp hds-hnas"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa1b9901_0f8ed2cf","line":10,"updated":"2015-08-20 18:55:58.000000000","message":"Why is returning a success a problem? Please better explain this.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"24b15a310442b3cea9380a0b55416584448a06f4","unresolved":false,"context_lines":[{"line_number":10,"context_line":"when the job is submitted. This patch fixes it by tracking the"},{"line_number":11,"context_line":"job status and reporting success only when the job succeeds."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Partially implements bp hds-hnas"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I20c692d19ea8afc6a14e9651c2af2c4de8cac553"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"da20952f_3fac7d9f","line":13,"updated":"2015-08-28 13:20:06.000000000","message":"It should be bugfix.","commit_id":"2a48ee7a213a67069444f415950bb3b7305275d2"}],"manila/share/drivers/hitachi/hds_hnas.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"72b08b3f86cc6c8e9d354f2820ebaa99c87673a7","unresolved":false,"context_lines":[{"line_number":61,"context_line":"               default\u003dNone,"},{"line_number":62,"context_line":"               help\u003d\"The IP of the clusters admin node. Only set in HNAS \""},{"line_number":63,"context_line":"                    \"multinode clusters.\"),"},{"line_number":64,"context_line":"    cfg.IntOpt(\u0027hds_hnas_job_timeout\u0027,"},{"line_number":65,"context_line":"               default\u003d30,"},{"line_number":66,"context_line":"               help\u003d\"The time to wait for stalled HNAS jobs before aborting.\"),"},{"line_number":67,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_dc0ada89","line":64,"updated":"2015-08-20 18:55:58.000000000","message":"please use a more meaningful name related to the use of the option: e.g. hds_hnas_stalled_timeout.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"72b08b3f86cc6c8e9d354f2820ebaa99c87673a7","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                    \"multinode clusters.\"),"},{"line_number":64,"context_line":"    cfg.IntOpt(\u0027hds_hnas_job_timeout\u0027,"},{"line_number":65,"context_line":"               default\u003d30,"},{"line_number":66,"context_line":"               help\u003d\"The time to wait for stalled HNAS jobs before aborting.\"),"},{"line_number":67,"context_line":"]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_8f9bc265","line":66,"updated":"2015-08-20 18:55:58.000000000","message":"Please explain that this is the timeout only if the copy is stalled.\n... time (in seconds) in...\nIt seems to me that 30s is a very small timeout.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"}],"manila/share/drivers/hitachi/ssh.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"b02fe1fc134717e7b2924b17c2e6df0d7cef105b","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                       src_path, snap_path]"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"            output, err \u003d self._execute(command)"},{"line_number":268,"context_line":"            if \u0027Request submitted successfully\u0027 in output:"},{"line_number":269,"context_line":"                result \u003d re.search(\u0027Job id \u003d (\\S*)\u0027, output)"},{"line_number":270,"context_line":"                job_id \u003d result.group(1)"},{"line_number":271,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_d27ac187","line":268,"updated":"2015-08-20 18:59:54.000000000","message":"This condition has no else, if \"Request submitted successfully\" is not in output you are letting the method finish successfully, it will return that the snapshot has been created successfully to the API. You should add an else throwing an exception, causing the methods in the stack to abort.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":17097,"name":"Tiago Pasqualini da Silva","email":"t.pasqualini@luizalabs.com","username":"tiago.pasqualini"},"change_message_id":"b388d94abf27cd80d299164edcced241cfd38599","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                       src_path, snap_path]"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"            output, err \u003d self._execute(command)"},{"line_number":268,"context_line":"            if \u0027Request submitted successfully\u0027 in output:"},{"line_number":269,"context_line":"                result \u003d re.search(\u0027Job id \u003d (\\S*)\u0027, output)"},{"line_number":270,"context_line":"                job_id \u003d result.group(1)"},{"line_number":271,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_121599e3","line":268,"in_reply_to":"fa1b9901_d27ac187","updated":"2015-08-20 19:12:00.000000000","message":"In this case, self._execute will raise an exception that will be caught on line 312.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"72b08b3f86cc6c8e9d354f2820ebaa99c87673a7","unresolved":false,"context_lines":[{"line_number":271,"context_line":""},{"line_number":272,"context_line":"                output \u003d \u0027\u0027"},{"line_number":273,"context_line":"                progress \u003d \u0027\u0027"},{"line_number":274,"context_line":"                tries \u003d 0"},{"line_number":275,"context_line":"                starttime \u003d time.time()"},{"line_number":276,"context_line":"                deadline \u003d starttime + self.job_timeout"},{"line_number":277,"context_line":"                while \u0027Job was completed\u0027 not in output:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_8f598237","line":274,"updated":"2015-08-20 18:55:58.000000000","message":"tries -\u003e job_rechecks","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":14907,"name":"Alyson","email":"alyson.rodrigues.rosa@gmail.com","username":"Alyson"},"change_message_id":"2b961647c1c160735cabce33d25a068e68ad5eef","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                    output, err \u003d self._execute(command)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"                    if \u0027Job failed\u0027 in output:"},{"line_number":282,"context_line":"                        LOG.error(_LE(\u0027Error in snapshot creation.\u0027))"},{"line_number":283,"context_line":"                        msg \u003d _(\u0027Share snapshot %s was not created.\u0027)"},{"line_number":284,"context_line":"                        raise exception.HNASBackendException(msg, snapshot_id)"},{"line_number":285,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_1cd612f1","line":282,"updated":"2015-08-20 18:34:23.000000000","message":"No snapshot identification. Use snapshot_id to log.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":14907,"name":"Alyson","email":"alyson.rodrigues.rosa@gmail.com","username":"Alyson"},"change_message_id":"2b961647c1c160735cabce33d25a068e68ad5eef","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                    if \u0027Job failed\u0027 in output:"},{"line_number":282,"context_line":"                        LOG.error(_LE(\u0027Error in snapshot creation.\u0027))"},{"line_number":283,"context_line":"                        msg \u003d _(\u0027Share snapshot %s was not created.\u0027)"},{"line_number":284,"context_line":"                        raise exception.HNASBackendException(msg, snapshot_id)"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"                    old_progress \u003d progress"},{"line_number":287,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_7c9e2e08","line":284,"updated":"2015-08-20 18:34:23.000000000","message":"This msg does not log the snapshot id correctly. You should do something like this:\n\nmsg \u003d (_(\"Share snapshot %s was not created. \") % snapshot_id)\n\nraise exception.HNASBackendException(msg\u003dmsg)","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":14907,"name":"Alyson","email":"alyson.rodrigues.rosa@gmail.com","username":"Alyson"},"change_message_id":"2b961647c1c160735cabce33d25a068e68ad5eef","unresolved":false,"context_lines":[{"line_number":298,"context_line":"                        if now \u003e deadline:"},{"line_number":299,"context_line":"                            command \u003d [\u0027tree-clone-job-abort\u0027, job_id]"},{"line_number":300,"context_line":"                            output, err \u003d self._execute(command)"},{"line_number":301,"context_line":"                            LOG.error(_LE(\"Timeout in snapshot creation.\"))"},{"line_number":302,"context_line":"                            msg \u003d _(\"Share snapshot %s was not created.\")"},{"line_number":303,"context_line":"                            raise exception.HNASBackendException(msg,"},{"line_number":304,"context_line":"                                                                 snapshot_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_9cb8e276","line":301,"updated":"2015-08-20 18:34:23.000000000","message":"No snapshot identification.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":14907,"name":"Alyson","email":"alyson.rodrigues.rosa@gmail.com","username":"Alyson"},"change_message_id":"2b961647c1c160735cabce33d25a068e68ad5eef","unresolved":false,"context_lines":[{"line_number":301,"context_line":"                            LOG.error(_LE(\"Timeout in snapshot creation.\"))"},{"line_number":302,"context_line":"                            msg \u003d _(\"Share snapshot %s was not created.\")"},{"line_number":303,"context_line":"                            raise exception.HNASBackendException(msg,"},{"line_number":304,"context_line":"                                                                 snapshot_id)"},{"line_number":305,"context_line":"                        else:"},{"line_number":306,"context_line":"                            time.sleep(tries ** 2)"},{"line_number":307,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_fca9de24","line":304,"updated":"2015-08-20 18:34:23.000000000","message":"This msg does not log the snapshot id correctly. You should do something like this:\n\nmsg \u003d (_(\"Share snapshot %s was not created. \") % snapshot_id)\n\nraise exception.HNASBackendException(msg\u003dmsg)","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":14907,"name":"Alyson","email":"alyson.rodrigues.rosa@gmail.com","username":"Alyson"},"change_message_id":"2b961647c1c160735cabce33d25a068e68ad5eef","unresolved":false,"context_lines":[{"line_number":307,"context_line":"                    else:"},{"line_number":308,"context_line":"                        tries \u003d 0"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"                LOG.debug(\"Snapshot created successfully.\")"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        except processutils.ProcessExecutionError as e:"},{"line_number":313,"context_line":"            if (\u0027Cannot find any clonable files in the source directory\u0027 in"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_fcda3e16","line":310,"updated":"2015-08-20 18:34:23.000000000","message":"No snapshot identification.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"72b08b3f86cc6c8e9d354f2820ebaa99c87673a7","unresolved":false,"context_lines":[{"line_number":307,"context_line":"                    else:"},{"line_number":308,"context_line":"                        tries \u003d 0"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"                LOG.debug(\"Snapshot created successfully.\")"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"        except processutils.ProcessExecutionError as e:"},{"line_number":313,"context_line":"            if (\u0027Cannot find any clonable files in the source directory\u0027 in"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_6fc98eab","line":310,"updated":"2015-08-20 18:55:58.000000000","message":"Please also log the IDs of the share and snapshot.","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"31139360c76aff1323b06fc8b4fafc4d74d75481","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                       src_path, snap_path]"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"            output, err \u003d self._execute(command)"},{"line_number":268,"context_line":"            if \u0027Request submitted successfully\u0027 in output:"},{"line_number":269,"context_line":"                result \u003d re.search(\u0027Job id \u003d (\\S*)\u0027, output)"},{"line_number":270,"context_line":"                job_id \u003d result.group(1)"},{"line_number":271,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_234aa355","line":268,"updated":"2015-08-21 13:28:35.000000000","message":"Actually, I believe you could have followed the pattern in this class by parsing the output into something like a \"TreeCloneJob\" class, it would look a lot nicer and easier to understand the code and maintain it.\n\nAlso, according to the output that can be seem in the unit test, it seems some other fields like \"source files missing\", \"source directories missing\" and \"source files skipped\" could be parsed to make sure that the snapshot is not missing any files, as the user would expect.\n\nI would also double check \"Job was completed\" with \"Success\" field, which you are not checking, I am not sure if it is possible to have a job completed with another status, but it looks like those fields should be have those values in order to consider that a snapshot has been created successfully.","commit_id":"9cf6e3a8697a611a9393e0fb08d15154c680bc42"},{"author":{"_account_id":11865,"name":"Clinton Knight","email":"cknight@netapp.com","username":"cknight"},"change_message_id":"4443c6196efcd2a7a50da41b27a2c05d67b16117","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                    old_progress \u003d progress"},{"line_number":295,"context_line":"                    progress \u003d job_status.data_bytes_processed"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"                    if old_progress \u003d\u003d progress:"},{"line_number":298,"context_line":"                        job_rechecks +\u003d 1"},{"line_number":299,"context_line":"                        now \u003d time.time()"},{"line_number":300,"context_line":"                        if now \u003e deadline:"}],"source_content_type":"text/x-python","patch_set":4,"id":"da20952f_f008d82a","line":297,"updated":"2015-08-26 22:38:18.000000000","message":"This block doesn\u0027t appear to be covered by any unit tests.  Can you add those?  And please don\u0027t forget to mock time.sleep, lest the unit tests take a long time.","commit_id":"23bda90a667830e070f2213b8f21a0103c0b2645"}],"manila/tests/share/drivers/hitachi/test_ssh.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"72b08b3f86cc6c8e9d354f2820ebaa99c87673a7","unresolved":false,"context_lines":[{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"        self.assertRaises(putils.ProcessExecutionError,"},{"line_number":1154,"context_line":"                          self._driver._mount, self.fs_name)"},{"line_number":1155,"context_line":"        ssh.HNASSSHBackend._execute.assert_called_with(fake_mount_command)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_2f107656","line":1155,"updated":"2015-08-20 18:55:58.000000000","message":"Extra tab??","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":17097,"name":"Tiago Pasqualini da Silva","email":"t.pasqualini@luizalabs.com","username":"tiago.pasqualini"},"change_message_id":"b388d94abf27cd80d299164edcced241cfd38599","unresolved":false,"context_lines":[{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"        self.assertRaises(putils.ProcessExecutionError,"},{"line_number":1154,"context_line":"                          self._driver._mount, self.fs_name)"},{"line_number":1155,"context_line":"        ssh.HNASSSHBackend._execute.assert_called_with(fake_mount_command)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa1b9901_121b59cb","line":1155,"in_reply_to":"fa1b9901_2f107656","updated":"2015-08-20 19:12:00.000000000","message":"Not applicable","commit_id":"dd93c744ffbc78436a3fc7e0dd63ac153dd727e5"},{"author":{"_account_id":14907,"name":"Alyson","email":"alyson.rodrigues.rosa@gmail.com","username":"Alyson"},"change_message_id":"323b1a9ee1f9749006d01ab9e43558e7696fd2c6","unresolved":false,"context_lines":[{"line_number":877,"context_line":"                          self.vvol, self.vvol[\u0027id\u0027])"},{"line_number":878,"context_line":"        ssh.HNASSSHBackend._execute.assert_called_with(fake_list_command)"},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"    def test_create_snapshot_happy_day(self):"},{"line_number":881,"context_line":"        fake_create_command \u003d [\u0027tree-clone-job-submit\u0027, \u0027-e\u0027,"},{"line_number":882,"context_line":"                               \u0027-f\u0027, \u0027file_system\u0027, \u0027/shares/vvol_test\u0027,"},{"line_number":883,"context_line":"                               \u0027/snapshots/vvol_test/snapshot_test\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_42e6b844","line":880,"updated":"2015-08-21 11:56:29.000000000","message":"This is the only function with \u0027happy day\u0027 in name, all others \"happy day\" functions has not. You could change this function to test_create_snapshot or change all others functions to include happy day in name.","commit_id":"9cf6e3a8697a611a9393e0fb08d15154c680bc42"},{"author":{"_account_id":11865,"name":"Clinton Knight","email":"cknight@netapp.com","username":"cknight"},"change_message_id":"d7cedd6b18dba75b0a25c02baa5c2964d8f00e0a","unresolved":false,"context_lines":[{"line_number":935,"context_line":""},{"line_number":936,"context_line":"    def test_create_snapshot_hnas_timeout(self):"},{"line_number":937,"context_line":"        self.mock_object(time, \"time\", mock.Mock(side_effect\u003d[1, 1, 200, 300]))"},{"line_number":938,"context_line":"        self.mock_object(time, \"sleep\", mock.Mock(return_value\u003d\"\"))"},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"        # Tests when a running tree job stalls at HNAS"},{"line_number":941,"context_line":"        self.mock_object(ssh.HNASSSHBackend, \u0027_execute\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"da20952f_6e72604b","line":938,"updated":"2015-08-28 02:41:57.000000000","message":"The default mock provided by mock_object is all you need here:\n\n    self.mock_object(time, \u0027sleep\u0027)","commit_id":"641acec492d4c818b168f34748cc3cd9fdfa3719"}]}