)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9746,"name":"Doug Shelley","email":"dougshelley66@gmail.com","username":"dougshelley"},"change_message_id":"b9dd3cf7b3d28bce8f5eb111067a114e1fe0436d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Denis Makogon \u003cdmakogon@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-06-24 19:02:57 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix backup state updating when guest error occures"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Reason:"},{"line_number":10,"context_line":"  - When guest error appears at \u0027create_backup\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1ae5cdf2_6bde24dc","line":7,"range":{"start_line":7,"start_character":43,"end_line":7,"end_character":50},"updated":"2014-06-24 19:35:33.000000000","message":"sic - \"occurs\"","commit_id":"1d75c7e138643790727b0d845cce0369c26c8e5d"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"c1e48e31db93ee51a2b9bc002bdfe7fd6747b48a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Denis Makogon \u003cdmakogon@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-07-08 13:10:55 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Update backup state when guest error occures"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Reason:"},{"line_number":10,"context_line":"  - When guest error appears at \u0027create_backup\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"baada198_7d74289c","line":7,"updated":"2014-07-10 01:33:29.000000000","message":"occurs has no \u0027e\u0027","commit_id":"a3db7d6214f375c3af207ba531f71915187c9fb1"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"d9c1921e6ea0bad34cd9afd35932fa6fc8cb4fe9","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Denis Makogon \u003cdmakogon@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-07-21 16:03:57 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Update backup state when guest error occures"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Reason:"},{"line_number":10,"context_line":"  - When guest error appears at \u0027create_backup\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"baada198_456466e4","line":7,"updated":"2014-07-21 16:28:02.000000000","message":"occurs","commit_id":"e5d633f209b7de185f0e93f231aa5a141dbfa5cd"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"c26021593e32ccced0237bd8bb3a334c471d43fa","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Denis Makogon \u003cdmakogon@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-07-21 16:03:57 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Update backup state when guest error occures"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Reason:"},{"line_number":10,"context_line":"  - When guest error appears at \u0027create_backup\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"baada198_a8ea790c","line":7,"in_reply_to":"baada198_456466e4","updated":"2014-07-21 16:44:21.000000000","message":"Done","commit_id":"e5d633f209b7de185f0e93f231aa5a141dbfa5cd"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"5315070f9ba5afb56adf1a4e883283908636b079","unresolved":false,"context_lines":[{"line_number":16,"context_line":"Changes:"},{"line_number":17,"context_line":"  - updating status when guest error appears;"},{"line_number":18,"context_line":"  - updating backup model with a new method;"},{"line_number":19,"context_line":"  - adding test;"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I05f5caea4380edbe0e8bebc618ecfb52afe45872"},{"line_number":22,"context_line":"Closes-Bug: #1333659"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"9aa7fdbe_50f6e15d","line":19,"updated":"2014-10-28 12:40:36.000000000","message":"no tests?","commit_id":"253f63c0d5fdc29b0814b4ceade85bea1d8acc4e"}],"trove/backup/models.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"cdfbae0c1c1ba1e4eccf98b24babd705e8331e4a","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from sqlalchemy import desc"},{"line_number":18,"context_line":"from swiftclient.client import ClientException"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from trove.common import cfg"},{"line_number":22,"context_line":"from trove.common import exception"},{"line_number":23,"context_line":"from trove.db.models import DatabaseModelBase"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_1c75f1f9","line":20,"updated":"2014-06-24 15:10:22.000000000","message":"why the extra line?","commit_id":"c00cd43fcf33dd57d0fbf83d8d6decea2cfe5280"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"cdfbae0c1c1ba1e4eccf98b24babd705e8331e4a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"from trove.common.remote import create_swift_client"},{"line_number":28,"context_line":"from trove.common import utils"},{"line_number":29,"context_line":"from trove.quota.quota import run_with_quotas"},{"line_number":30,"context_line":"from trove.openstack.common.gettextutils import _"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"CONF \u003d cfg.CONF"},{"line_number":33,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_1c4c11c8","line":30,"updated":"2014-06-24 15:10:22.000000000","message":"Out of scope - this should probably be done in context of the logging message fixes","commit_id":"c00cd43fcf33dd57d0fbf83d8d6decea2cfe5280"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"f30d80110fb74fc42519e11479f7eb9747024ea3","unresolved":false,"context_lines":[{"line_number":27,"context_line":"from trove.common.remote import create_swift_client"},{"line_number":28,"context_line":"from trove.common import utils"},{"line_number":29,"context_line":"from trove.quota.quota import run_with_quotas"},{"line_number":30,"context_line":"from trove.openstack.common.gettextutils import _"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"CONF \u003d cfg.CONF"},{"line_number":33,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_0d8f7d77","line":30,"in_reply_to":"1ae5cdf2_1c4c11c8","updated":"2014-06-24 15:59:18.000000000","message":"If you look at models.py you will see that this import was missed and this fix fits to this patch","commit_id":"c00cd43fcf33dd57d0fbf83d8d6decea2cfe5280"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"17ee241fe3c9b0f70120a078280110f048d1ab83","unresolved":false,"context_lines":[{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    @classmethod"},{"line_number":328,"context_line":"    def update_backup(cls, backup_id, **values):"},{"line_number":329,"context_line":"        backup \u003d DBBackup.get_by(id\u003dbackup_id, deleted\u003dFalse)"},{"line_number":330,"context_line":"        backup.update(**values)"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_2e6bc6b7","line":329,"updated":"2014-07-07 15:16:24.000000000","message":"if you are getting a backup_id, isn\u0027t deleted\u003dFalse redundant?\n\nIs there a reason you don\u0027t use DBBackup.get_by_id()?","commit_id":"28cb8ede0927d8d7815272f42a72fbcaeffdd1af"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"439b9296c9a9e23d38ffbfe7a8fad11d87b23971","unresolved":false,"context_lines":[{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    @classmethod"},{"line_number":328,"context_line":"    def update_backup(cls, backup_id, **values):"},{"line_number":329,"context_line":"        backup \u003d DBBackup.get_by(id\u003dbackup_id, deleted\u003dFalse)"},{"line_number":330,"context_line":"        backup.update(**values)"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_2ffbc147","line":329,"in_reply_to":"baada198_2e6bc6b7","updated":"2014-07-07 16:00:15.000000000","message":"First of all, method get_by_id is a method of Backup class.\nSecond, take a look at #1. I don\u0027t want to jump between classes (Backup vs. DBBackup).\nThird, get_by_id does the same as find_by, get_by, etc. At this stage backup record exists, 100%, no need to wrap it with additional try:except block.","commit_id":"28cb8ede0927d8d7815272f42a72fbcaeffdd1af"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"930b35a4c430a50882b55510f8f8680a1f55e041","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                raise exception.SwiftAuthError(tenant_id\u003dcontext.tenant)"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    @classmethod"},{"line_number":333,"context_line":"    def update_backup(cls, backup_id, **values):"},{"line_number":334,"context_line":"        backup \u003d DBBackup.get_by(id\u003dbackup_id, deleted\u003dFalse)"},{"line_number":335,"context_line":"        backup.update(**values)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_ad2a012a","line":333,"updated":"2014-09-01 21:23:45.000000000","message":"why the redundant \"_backup\" in the name? \n\nSince the class is DBBackup, just update should suffice.","commit_id":"b37adef9e5396d09b80f1c0b305edd0d1ba5555f"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"15eb8b5542f6e6cead882a901cfd985d56709b56","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                raise exception.SwiftAuthError(tenant_id\u003dcontext.tenant)"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    @classmethod"},{"line_number":333,"context_line":"    def update_backup(cls, backup_id, **values):"},{"line_number":334,"context_line":"        backup \u003d DBBackup.get_by(id\u003dbackup_id, deleted\u003dFalse)"},{"line_number":335,"context_line":"        backup.update(**values)"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_4d4c0e43","line":333,"in_reply_to":"fa98f980_ad2a012a","updated":"2014-09-02 14:32:55.000000000","message":"Done","commit_id":"b37adef9e5396d09b80f1c0b305edd0d1ba5555f"}],"trove/taskmanager/models.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"cdfbae0c1c1ba1e4eccf98b24babd705e8331e4a","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"class ExceptionStatusHandlerMixin(object):"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _log_messages(self, exc, message):"},{"line_number":139,"context_line":"        LOG.error(message)"},{"line_number":140,"context_line":"        LOG.error(exc)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_9c3901f5","line":137,"updated":"2014-06-24 15:10:22.000000000","message":"How about adding a comment here?  I know all classes here don\u0027t have one, but it\u0027s probably a good thing to add for new classes.","commit_id":"c00cd43fcf33dd57d0fbf83d8d6decea2cfe5280"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"8a3d89fa2572bf5d88b1cfb683f436901f3ba2f1","unresolved":false,"context_lines":[{"line_number":514,"context_line":"        LOG.error(traceback.format_exc())"},{"line_number":515,"context_line":"        self.update_db(task_status\u003dtask_status)"},{"line_number":516,"context_line":"        raise TroveError(message\u003dmessage)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"    def _create_volume(self, volume_size, datastore_manager):"},{"line_number":519,"context_line":"        LOG.info(\"Entering create_volume\")"},{"line_number":520,"context_line":"        LOG.debug(\"begin _create_volume for id: %s\" % self.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_1d394fa3","side":"PARENT","line":517,"updated":"2014-06-25 09:30:30.000000000","message":"Why are we refactoring this part of the codebase when it is not relevant to fixing the big in question?","commit_id":"46a2f7d377bde1fe1b7b80b3be640eadeb44a4de"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"8a3d89fa2572bf5d88b1cfb683f436901f3ba2f1","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                        payload)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"class ExceptionStatusHandlerMixin(object):"},{"line_number":137,"context_line":"    \"\"\""},{"line_number":138,"context_line":"    Contains method that are useful for logging"},{"line_number":139,"context_line":"    and raising errors for different cases"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_9d70ffdb","line":136,"updated":"2014-06-25 09:30:30.000000000","message":"Having a separate ExceptionStatusHandlerMixin just to handle an exception in case a backup fails seems a bit overkill.","commit_id":"1d75c7e138643790727b0d845cce0369c26c8e5d"},{"author":{"_account_id":9746,"name":"Doug Shelley","email":"dougshelley66@gmail.com","username":"dougshelley"},"change_message_id":"b9dd3cf7b3d28bce8f5eb111067a114e1fe0436d","unresolved":false,"context_lines":[{"line_number":798,"context_line":"        try:"},{"line_number":799,"context_line":"            self.guest.create_backup(backup_info)"},{"line_number":800,"context_line":"        except (GuestTimeout, GuestError) as ex:"},{"line_number":801,"context_line":"            msg \u003d _(\"Exception happend while creating \""},{"line_number":802,"context_line":"                    \"backup for instance %s\") % self.id"},{"line_number":803,"context_line":"            self.reset_task_status()"},{"line_number":804,"context_line":"            self._mark_backup_as_failed(ex, msg, backup_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_0bd920f4","line":801,"range":{"start_line":801,"start_character":31,"end_line":801,"end_character":38},"updated":"2014-06-24 19:35:33.000000000","message":"sic - should be \"happened\"","commit_id":"1d75c7e138643790727b0d845cce0369c26c8e5d"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"193a702e6fdb27a598f150bbc8c400045e3b491c","unresolved":false,"context_lines":[{"line_number":787,"context_line":"                    \"backup for instance %s\") % self.id"},{"line_number":788,"context_line":"            LOG.error(msg)"},{"line_number":789,"context_line":"            LOG.error(ex)"},{"line_number":790,"context_line":"            LOG.error(traceback.format_exc())"},{"line_number":791,"context_line":"            self.reset_task_status()"},{"line_number":792,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":793,"context_line":"                backup_info[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_8bcdb668","line":790,"updated":"2014-07-07 05:16:09.000000000","message":"LOG.exception should help simplify this.","commit_id":"659f4b4fad8b4670cfbedc6c62fe881edf42e3fc"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"16a4798858ced7a7fc228a29263a454acffba1b7","unresolved":false,"context_lines":[{"line_number":787,"context_line":"                    \"backup for instance %s\") % self.id"},{"line_number":788,"context_line":"            LOG.error(msg)"},{"line_number":789,"context_line":"            LOG.error(ex)"},{"line_number":790,"context_line":"            LOG.error(traceback.format_exc())"},{"line_number":791,"context_line":"            self.reset_task_status()"},{"line_number":792,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":793,"context_line":"                backup_info[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_287ae9bc","line":790,"in_reply_to":"baada198_8bcdb668","updated":"2014-07-07 09:28:42.000000000","message":"Done","commit_id":"659f4b4fad8b4670cfbedc6c62fe881edf42e3fc"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"193a702e6fdb27a598f150bbc8c400045e3b491c","unresolved":false,"context_lines":[{"line_number":792,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":793,"context_line":"                backup_info[\u0027id\u0027],"},{"line_number":794,"context_line":"                state\u003dbkup_models.BackupState.FAILED)"},{"line_number":795,"context_line":"            raise ex"},{"line_number":796,"context_line":""},{"line_number":797,"context_line":"    def reboot(self):"},{"line_number":798,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_0be1a6d0","line":795,"updated":"2014-07-07 05:16:09.000000000","message":"Do not re-raise ex, since that will clobber the old traceback. \n\nSimply \"raise\" instead.","commit_id":"659f4b4fad8b4670cfbedc6c62fe881edf42e3fc"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"16a4798858ced7a7fc228a29263a454acffba1b7","unresolved":false,"context_lines":[{"line_number":792,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":793,"context_line":"                backup_info[\u0027id\u0027],"},{"line_number":794,"context_line":"                state\u003dbkup_models.BackupState.FAILED)"},{"line_number":795,"context_line":"            raise ex"},{"line_number":796,"context_line":""},{"line_number":797,"context_line":"    def reboot(self):"},{"line_number":798,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_48755dc9","line":795,"in_reply_to":"baada198_0be1a6d0","updated":"2014-07-07 09:28:42.000000000","message":"Done","commit_id":"659f4b4fad8b4670cfbedc6c62fe881edf42e3fc"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"17ee241fe3c9b0f70120a078280110f048d1ab83","unresolved":false,"context_lines":[{"line_number":787,"context_line":"            msg \u003d (_(\"Exception happend while creating \""},{"line_number":788,"context_line":"                   \"backup for instance %(id)s. Original exception: %(ex)s\") %"},{"line_number":789,"context_line":"                   {\u0027id\u0027: self.id, \u0027ex\u0027: str(ex)})"},{"line_number":790,"context_line":"            LOG.exception(msg)"},{"line_number":791,"context_line":"            self.reset_task_status()"},{"line_number":792,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":793,"context_line":"                backup_info[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_d331115b","line":790,"updated":"2014-07-07 15:16:24.000000000","message":"1. Why not just LOG.exception(str(ex))\n\n2. If you must have a rendered string here:\n\n(a) happenEd is spelt with two e\u0027s\n\n(b) it would be preferable to either drop the word happened or replace it with occurred\n\n(c) the useful thing out of this message would be the backup id (which you are going to mark FAILED). It would be good to emit that as well\n\n(d) include in your rendered message that the backup will be marked FAILED.","commit_id":"28cb8ede0927d8d7815272f42a72fbcaeffdd1af"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"439b9296c9a9e23d38ffbfe7a8fad11d87b23971","unresolved":false,"context_lines":[{"line_number":787,"context_line":"            msg \u003d (_(\"Exception happend while creating \""},{"line_number":788,"context_line":"                   \"backup for instance %(id)s. Original exception: %(ex)s\") %"},{"line_number":789,"context_line":"                   {\u0027id\u0027: self.id, \u0027ex\u0027: str(ex)})"},{"line_number":790,"context_line":"            LOG.exception(msg)"},{"line_number":791,"context_line":"            self.reset_task_status()"},{"line_number":792,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":793,"context_line":"                backup_info[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_4f7b95bd","line":790,"in_reply_to":"baada198_d331115b","updated":"2014-07-07 16:00:15.000000000","message":"no need to write in exception message that backup is going to be marked as FAILED, because next call to method save(of DatabaseModelBase) will do that.","commit_id":"28cb8ede0927d8d7815272f42a72fbcaeffdd1af"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"2419b9f3878e08bf50047e42048a0c2fa04783a4","unresolved":false,"context_lines":[{"line_number":788,"context_line":"                   \"backup for instance %(id)s. Backup info: %(backup)s\") %"},{"line_number":789,"context_line":"                   {\u0027id\u0027: self.id, \u0027backup\u0027: backup_info})"},{"line_number":790,"context_line":"            LOG.exception(msg)"},{"line_number":791,"context_line":"            LOG.exception(str(ex))"},{"line_number":792,"context_line":"            self.reset_task_status()"},{"line_number":793,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":794,"context_line":"                backup_info[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_3830499e","line":791,"updated":"2014-07-08 10:46:42.000000000","message":"maybe you made this change based on my comments on patch set #10. I didn\u0027t intend that you would add two LOG.exception() calls. If you positively must have this string that you are tacking on here, then I\u0027d make it all a single call to LOG.exception()\n\nAlso, are you printing the entire backup_info? Wouldn\u0027t the backup id (backup_info[\u0027id\u0027]) suffice? After all, that\u0027s the thing a person would need to go and find/delete the backup. \n\nI actually don\u0027t even know what printing the complete backup info would do.","commit_id":"a3db7d6214f375c3af207ba531f71915187c9fb1"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"028feb2d508c1e8b6d3d20379e99a3905601a8f9","unresolved":false,"context_lines":[{"line_number":788,"context_line":"                   \"backup for instance %(id)s. Backup info: %(backup)s\") %"},{"line_number":789,"context_line":"                   {\u0027id\u0027: self.id, \u0027backup\u0027: backup_info})"},{"line_number":790,"context_line":"            LOG.exception(msg)"},{"line_number":791,"context_line":"            LOG.exception(str(ex))"},{"line_number":792,"context_line":"            self.reset_task_status()"},{"line_number":793,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":794,"context_line":"                backup_info[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_9b930f5c","line":791,"in_reply_to":"baada198_3830499e","updated":"2014-07-08 11:12:23.000000000","message":"I decided to have two exception loggers, one for custom message, one for actual exception. That\u0027s in, one logging call is not enough.","commit_id":"a3db7d6214f375c3af207ba531f71915187c9fb1"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"679721d4784c6695cb2cf41933f606a43b0abc08","unresolved":false,"context_lines":[{"line_number":807,"context_line":"                   \"backup for instance %(id)s. Backup info: %(backup)s\") %"},{"line_number":808,"context_line":"                   {\u0027id\u0027: self.id, \u0027backup\u0027: backup_info})"},{"line_number":809,"context_line":"            LOG.exception(msg)"},{"line_number":810,"context_line":"            LOG.exception(str(ex))"},{"line_number":811,"context_line":"            self.reset_task_status()"},{"line_number":812,"context_line":"            bkup_models.DBBackup.update_backup("},{"line_number":813,"context_line":"                backup_info[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_c7afaf29","line":810,"updated":"2014-07-21 09:00:59.000000000","message":"LOG.exception automagically logs the exception message and traceback, so this second LOG line is not needed.","commit_id":"99e80a87cb86397232237273b5cb0880a6ed92de"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"d9c1921e6ea0bad34cd9afd35932fa6fc8cb4fe9","unresolved":false,"context_lines":[{"line_number":802,"context_line":"        LOG.debug(\"Calling create_backup  %s \" % self.id)"},{"line_number":803,"context_line":"        try:"},{"line_number":804,"context_line":"            self.guest.create_backup(backup_info)"},{"line_number":805,"context_line":"        except (GuestTimeout, GuestError):"},{"line_number":806,"context_line":"            msg \u003d (_(\"Exception occurred while creating \""},{"line_number":807,"context_line":"                   \"backup for instance %(id)s. Backup info: %(backup)s\") %"},{"line_number":808,"context_line":"                   {\u0027id\u0027: self.id, \u0027backup\u0027: backup_info})"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_a5d46220","line":805,"updated":"2014-07-21 16:28:02.000000000","message":"create_backup() can cause other exceptions, can it not? I think this handler should be widened","commit_id":"e5d633f209b7de185f0e93f231aa5a141dbfa5cd"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"c26021593e32ccced0237bd8bb3a334c471d43fa","unresolved":false,"context_lines":[{"line_number":802,"context_line":"        LOG.debug(\"Calling create_backup  %s \" % self.id)"},{"line_number":803,"context_line":"        try:"},{"line_number":804,"context_line":"            self.guest.create_backup(backup_info)"},{"line_number":805,"context_line":"        except (GuestTimeout, GuestError):"},{"line_number":806,"context_line":"            msg \u003d (_(\"Exception occurred while creating \""},{"line_number":807,"context_line":"                   \"backup for instance %(id)s. Backup info: %(backup)s\") %"},{"line_number":808,"context_line":"                   {\u0027id\u0027: self.id, \u0027backup\u0027: backup_info})"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_e879d182","line":805,"in_reply_to":"baada198_a5d46220","updated":"2014-07-21 16:44:21.000000000","message":"at this stage only two exceptions can happen - guest timeout, guest unavailability, nothing else since `create_backup` method is a `cast` type method, that\u0027s why we don\u0027t expect any response.","commit_id":"e5d633f209b7de185f0e93f231aa5a141dbfa5cd"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"5315070f9ba5afb56adf1a4e883283908636b079","unresolved":false,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    def create_backup(self, backup_info):"},{"line_number":906,"context_line":"        LOG.info(_(\"Initiating backup for instance %s.\") % self.id)"},{"line_number":907,"context_line":"        LOG.debug(\"Calling create_backup  %s \" % self.id)"},{"line_number":908,"context_line":"        try:"},{"line_number":909,"context_line":"            self.guest.create_backup(backup_info)"},{"line_number":910,"context_line":"        except (GuestTimeout, GuestError):"}],"source_content_type":"text/x-python","patch_set":28,"id":"9aa7fdbe_f0dd15e1","line":907,"updated":"2014-10-28 12:40:36.000000000","message":"Do we need two messages logged here?","commit_id":"253f63c0d5fdc29b0814b4ceade85bea1d8acc4e"},{"author":{"_account_id":10725,"name":"Sergey Gotliv","email":"sgotliv@redhat.com","username":"sgotliv"},"change_message_id":"df4b729cc3799fe47535845c62c63cb6ff00cdba","unresolved":false,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    def create_backup(self, backup_info):"},{"line_number":906,"context_line":"        LOG.info(_(\"Initiating backup for instance %s.\") % self.id)"},{"line_number":907,"context_line":"        LOG.debug(\"Calling create_backup  %s \" % self.id)"},{"line_number":908,"context_line":"        try:"},{"line_number":909,"context_line":"            self.guest.create_backup(backup_info)"},{"line_number":910,"context_line":"        except (GuestTimeout, GuestError):"}],"source_content_type":"text/x-python","patch_set":28,"id":"9aa7fdbe_b0c9cdad","line":907,"updated":"2014-10-28 12:18:33.000000000","message":"Redundant log printing.","commit_id":"253f63c0d5fdc29b0814b4ceade85bea1d8acc4e"}],"trove/tests/unittests/taskmanager/test_models.py":[{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"1baf4edcd4164b254c255efc421d2a39823bbb5b","unresolved":false,"context_lines":[{"line_number":559,"context_line":"            do_not_start_on_reboot\u003dTrue)"},{"line_number":560,"context_line":"        orig_server.resize.assert_any_call(self.new_flavor[\u0027id\u0027])"},{"line_number":561,"context_line":"        self.assertThat(self.db_instance.task_status, Is(InstanceTasks.NONE))"},{"line_number":562,"context_line":"        self.assertEqual(self.stub_server_mgr.get.call_count, 1)"},{"line_number":563,"context_line":"        self.assertThat(self.db_instance.flavor_id, Is(self.new_flavor[\u0027id\u0027]))"},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    def test_resize_flavor_resize_failure(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_f33c4b31","side":"PARENT","line":562,"updated":"2014-07-07 18:44:06.000000000","message":"Why was this check removed? Do resize_flavor not call the nova client anymore?","commit_id":"5c0b202ae347680e63853a592a26a6877e5405ba"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"7cb9168c826820f08fe0ffaee885b7d671015ee7","unresolved":false,"context_lines":[{"line_number":559,"context_line":"            do_not_start_on_reboot\u003dTrue)"},{"line_number":560,"context_line":"        orig_server.resize.assert_any_call(self.new_flavor[\u0027id\u0027])"},{"line_number":561,"context_line":"        self.assertThat(self.db_instance.task_status, Is(InstanceTasks.NONE))"},{"line_number":562,"context_line":"        self.assertEqual(self.stub_server_mgr.get.call_count, 1)"},{"line_number":563,"context_line":"        self.assertThat(self.db_instance.flavor_id, Is(self.new_flavor[\u0027id\u0027]))"},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    def test_resize_flavor_resize_failure(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_8232d7a4","side":"PARENT","line":562,"in_reply_to":"baada198_f33c4b31","updated":"2014-07-07 21:14:11.000000000","message":"+1. even if it does not, this should not be introduced in this patch-set.","commit_id":"5c0b202ae347680e63853a592a26a6877e5405ba"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"930b35a4c430a50882b55510f8f8680a1f55e041","unresolved":false,"context_lines":[{"line_number":619,"context_line":"                        self.instance_task.create_backup, backup_info)"},{"line_number":620,"context_line":"                    bckp \u003d backup_models.DBBackup.get_by(id\u003dbackup_info[\u0027id\u0027])"},{"line_number":621,"context_line":"                    self.assertEqual(backup_models.BackupState.FAILED,"},{"line_number":622,"context_line":"                                     bckp.state)"},{"line_number":623,"context_line":""},{"line_number":624,"context_line":""},{"line_number":625,"context_line":"class BackupTasksTest(testtools.TestCase):"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa98f980_cdaf45e2","line":622,"updated":"2014-09-01 21:23:45.000000000","message":"What is this unit test exactly testing? \n\nLooks like the two asserts that are being made by the test (i.e. GuestTimeout exception, and BackupState.FAILED) are both being set up in your mocks above. Looks to me like none of the actual Trove code is being tested here.","commit_id":"b37adef9e5396d09b80f1c0b305edd0d1ba5555f"},{"author":{"_account_id":4463,"name":"iccha-sethi","email":"iccha.sethi@rackspace.com","username":"iccha-sethi"},"change_message_id":"a52a4f42b62c6ca8241ad098c74affe335042682","unresolved":false,"context_lines":[{"line_number":495,"context_line":"        self.container_content \u003d (None,"},{"line_number":496,"context_line":"                                  [{\u0027name\u0027: \u0027first\u0027},"},{"line_number":497,"context_line":"                                   {\u0027name\u0027: \u0027second\u0027},"},{"line_number":498,"context_line":"                                   {\u0027name\u0027: \u0027third\u0027}])"},{"line_number":499,"context_line":"        # this is used during the final check of whether the resize successful"},{"line_number":500,"context_line":"        db_instance.server_status \u003d \u0027ACTIVE\u0027"},{"line_number":501,"context_line":"        self.db_instance \u003d db_instance"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa98f980_288a181b","line":498,"updated":"2014-09-03 20:21:21.000000000","message":"This is just adding it to the setup. There is no test to assert backup being marked as failure.","commit_id":"f50204a82320430848332b13311e03d164cb14a4"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"5315070f9ba5afb56adf1a4e883283908636b079","unresolved":false,"context_lines":[{"line_number":480,"context_line":"                                 tenant_id\u003d\u0027testresize-tenant-id\u0027,"},{"line_number":481,"context_line":"                                 volume_size\u003d\u00271\u0027,"},{"line_number":482,"context_line":"                                 volume_id\u003dVOLUME_ID)"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"        # this is used during the final check of whether the resize successful"},{"line_number":485,"context_line":"        db_instance.server_status \u003d \u0027ACTIVE\u0027"},{"line_number":486,"context_line":"        self.db_instance \u003d db_instance"}],"source_content_type":"text/x-python","patch_set":28,"id":"9aa7fdbe_d0ead172","side":"PARENT","line":483,"updated":"2014-10-28 12:40:36.000000000","message":"seems like a redundant whitespace change?","commit_id":"108fbc5ef388e9718e74f9136367f44f4ca375e8"}]}