)]}'
{"trove/guestagent/strategies/backup/mysql_impl.py":[{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"2d2750b3cbe89ceb08db6e80718cbe5270706615","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        LOG.debug(\u0027Checking mysqldump process output.\u0027)"},{"line_number":50,"context_line":"        with open(\u0027/tmp/mysqldump.log\u0027, \u0027r\u0027) as backup_log:"},{"line_number":51,"context_line":"            output \u003d backup_log.read()"},{"line_number":52,"context_line":"            LOG.info(output)"},{"line_number":53,"context_line":"            if not output:"},{"line_number":54,"context_line":"                LOG.error(_(\"Mysqldump log file empty.\"))"},{"line_number":55,"context_line":"                return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_703a0335","line":52,"range":{"start_line":52,"start_character":12,"end_line":52,"end_character":28},"updated":"2018-03-20 01:05:35.000000000","message":"Should we log all output in info level or using LOG.debug would be better, I guess? And I prefer checking if not output first then doing LOG.info(output).","commit_id":"22d16b6f5faf8352c8cd53b40a7485ebaca5ba97"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"477b25546968ddab0218954e7fc4815297c57dfa","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        LOG.debug(\u0027Checking mysqldump process output.\u0027)"},{"line_number":50,"context_line":"        with open(\u0027/tmp/mysqldump.log\u0027, \u0027r\u0027) as backup_log:"},{"line_number":51,"context_line":"            output \u003d backup_log.read()"},{"line_number":52,"context_line":"            LOG.info(output)"},{"line_number":53,"context_line":"            if not output:"},{"line_number":54,"context_line":"                LOG.error(_(\"Mysqldump log file empty.\"))"},{"line_number":55,"context_line":"                return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_75dcfa17","line":52,"range":{"start_line":52,"start_character":12,"end_line":52,"end_character":28},"in_reply_to":"df7087c5_38ef7388","updated":"2018-03-21 01:34:23.000000000","message":"Agree with Fan Zhang, it\u0027s better to be debugging logs and should only be logged after checking the backup_log file is not empty.","commit_id":"22d16b6f5faf8352c8cd53b40a7485ebaca5ba97"},{"author":{"_account_id":26970,"name":"Yang Youseok","email":"ileixe@gmail.com","username":"ileixe"},"change_message_id":"7e1940f24bcda8072d9c6c7a82120081a4286c9f","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        LOG.debug(\u0027Checking mysqldump process output.\u0027)"},{"line_number":50,"context_line":"        with open(\u0027/tmp/mysqldump.log\u0027, \u0027r\u0027) as backup_log:"},{"line_number":51,"context_line":"            output \u003d backup_log.read()"},{"line_number":52,"context_line":"            LOG.info(output)"},{"line_number":53,"context_line":"            if not output:"},{"line_number":54,"context_line":"                LOG.error(_(\"Mysqldump log file empty.\"))"},{"line_number":55,"context_line":"                return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"df7087c5_38ef7388","line":52,"range":{"start_line":52,"start_character":12,"end_line":52,"end_character":28},"in_reply_to":"df7087c5_703a0335","updated":"2018-03-20 08:31:46.000000000","message":"Yes, I think so, too. These messages seem to be related to debug not to be informative. Should I change them then?","commit_id":"22d16b6f5faf8352c8cd53b40a7485ebaca5ba97"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"34dc6d50834cb974a843e8b42f3a24ce7e5b653a","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        LOG.debug(\u0027Checking mysqldump process output.\u0027)"},{"line_number":50,"context_line":"        with open(\u0027/tmp/mysqldump.log\u0027, \u0027r\u0027) as backup_log:"},{"line_number":51,"context_line":"            output \u003d backup_log.read()"},{"line_number":52,"context_line":"            if not output:"},{"line_number":53,"context_line":"                LOG.error(\"Mysqldump log file empty.\")"},{"line_number":54,"context_line":"                return False"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            LOG.debug(output)"},{"line_number":57,"context_line":"            for line in output.splitlines():"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_76dc3346","line":54,"range":{"start_line":52,"start_character":0,"end_line":54,"end_character":28},"updated":"2018-03-26 02:04:04.000000000","message":"This may not be correct. Normally we would think everything is OK if there\u0027s nothing in the output(stdout and stderr). This is not the same as the case of InnoBackupEx, \u0027completed OK!\u0027 is expected there to ensure the backup is sucessfully completed.","commit_id":"ef61b2dfa3faadeace87ff737bf98aaff941d78f"},{"author":{"_account_id":26970,"name":"Yang Youseok","email":"ileixe@gmail.com","username":"ileixe"},"change_message_id":"b3fb81af5214e1df8adebf40e21757091d2c0de3","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        LOG.debug(\u0027Checking mysqldump process output.\u0027)"},{"line_number":50,"context_line":"        with open(\u0027/tmp/mysqldump.log\u0027, \u0027r\u0027) as backup_log:"},{"line_number":51,"context_line":"            output \u003d backup_log.read()"},{"line_number":52,"context_line":"            if not output:"},{"line_number":53,"context_line":"                LOG.error(\"Mysqldump log file empty.\")"},{"line_number":54,"context_line":"                return False"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"            LOG.debug(output)"},{"line_number":57,"context_line":"            for line in output.splitlines():"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_542b2893","line":54,"range":{"start_line":52,"start_character":0,"end_line":54,"end_character":28},"in_reply_to":"bf659307_76dc3346","updated":"2018-04-01 04:21:45.000000000","message":"Exactly. I was silly just to follow existed code. Thanks I will fix it.","commit_id":"ef61b2dfa3faadeace87ff737bf98aaff941d78f"}],"trove/tests/unittests/backup/test_backupagent.py":[{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"35c275a780d5a0ad6aee54fd5fad88007df29903","unresolved":false,"context_lines":[{"line_number":567,"context_line":"                           \u0027on the command line interface can be insecure.\u0027"},{"line_number":568,"context_line":"        str_will_be_false \u003d \u0027ERROR: mysqldump command did not succeed.\u0027"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"        with mock.patch(\u0027trove.guestagent.strategies.backup.mysql_impl.open\u0027,"},{"line_number":571,"context_line":"                        mock.mock_open(read_data\u003d\u0027\u0027)):"},{"line_number":572,"context_line":"            self.assertTrue(mysql_dump.check_process())"},{"line_number":573,"context_line":"        with mock.patch(\u0027trove.guestagent.strategies.backup.mysql_impl.open\u0027,"},{"line_number":574,"context_line":"                        mock.mock_open(read_data\u003dstr_will_be_true)):"},{"line_number":575,"context_line":"            self.assertTrue(mysql_dump.check_process())"},{"line_number":576,"context_line":"        with mock.patch(\u0027trove.guestagent.strategies.backup.mysql_impl.open\u0027,"},{"line_number":577,"context_line":"                        mock.mock_open(read_data\u003dstr_will_be_false)):"},{"line_number":578,"context_line":"            self.assertFalse(mysql_dump.check_process())"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f6a8fd7_5096724e","line":578,"range":{"start_line":570,"start_character":8,"end_line":578,"end_character":56},"updated":"2018-04-20 04:26:07.000000000","message":"This part may be the typical usage of ddt testing, though I\u0027m personally fine with the current approach of multiple \u0027with mock.patch\u0027.","commit_id":"88464d89612ae8150c73db52a4e3555961853fc1"}]}