)]}'
{"trove/instance/models.py":[{"author":{"_account_id":9746,"name":"Doug Shelley","email":"dougshelley66@gmail.com","username":"dougshelley"},"change_message_id":"cad84488c3e57d1b558f0a041c5f4a423f206a02","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                self.context, self.db_info.id)"},{"line_number":511,"context_line":"            for backup in backups:"},{"line_number":512,"context_line":"                if backup.is_running:"},{"line_number":513,"context_line":"                    msg \u003d {"},{"line_number":514,"context_line":"                        \u0027b\u0027: backup.id,"},{"line_number":515,"context_line":"                        \u0027i\u0027: self.db_info.id"},{"line_number":516,"context_line":"                    }"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_a6f3906b","line":513,"updated":"2014-06-27 12:30:01.000000000","message":"This msg/LOG.debug sequence seems backwards to what is done elsewhere in the code base. i.e.:\nmsg \u003d \"Updating backup %(backup_id)s for instance %(instance_id)s\"\nLOG.debug(msg % { \u0027backup_id\u0027: backup.id, \u0027instance_id\u0027: self.db_info.id } )\n\nI also don\u0027t think it is worthwhile to shorten those dict lookups to \u0027b\u0027 and \u0027i\u0027","commit_id":"3961f1b7d2d4f386e6dd437091468cba739b509c"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"4220a7ca444dafe64828d6c4de1aefb0220b6892","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                if backup.is_running:"},{"line_number":513,"context_line":"                    LOG.debug(\"Updating backup %s for instance %s\" % ("},{"line_number":514,"context_line":"                        backup.id, self.db_info.id))"},{"line_number":515,"context_line":"                    backup.update(state\u003dbackup_models.BackupState.FAILED)"},{"line_number":516,"context_line":"            task_api.API(self.context).delete_instance(self.id)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"        deltas \u003d {\u0027instances\u0027: -1}"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_45c7bb64","line":515,"updated":"2014-07-08 23:15:12.000000000","message":"I am not a big fan of this. I dont like how \u0027instance delete\u0027 has to be responsible of other things like backups, or to a greater extend, quota. Ideally, we would want other components to take care of these. However, i understand that this work would be massive and out of scope. So i am fine with this.","commit_id":"b73726aff0ba29775dc534cf54aec099b1240cd9"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"f90dd2587592644f7542e44d7ae3ca895be0ebe7","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                if backup.is_running:"},{"line_number":513,"context_line":"                    LOG.debug(\"Updating backup %s for instance %s\" % ("},{"line_number":514,"context_line":"                        backup.id, self.db_info.id))"},{"line_number":515,"context_line":"                    backup.update(state\u003dbackup_models.BackupState.FAILED)"},{"line_number":516,"context_line":"            task_api.API(self.context).delete_instance(self.id)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"        deltas \u003d {\u0027instances\u0027: -1}"}],"source_content_type":"text/x-python","patch_set":22,"id":"baada198_02c1952e","line":515,"updated":"2014-07-14 09:12:42.000000000","message":"This should not be done in the API. This should be done with the rest of the \"cleanup\" as part of the models / tasks that get called by the Taskmanager.","commit_id":"45d4a615f154817b2cd15ea75552e659571ee7f1"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"e489f97c84d98ea33fbae30e29f1bfe3734ba249","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                if backup.is_running:"},{"line_number":513,"context_line":"                    LOG.debug(\"Updating backup %s for instance %s\" % ("},{"line_number":514,"context_line":"                        backup.id, self.db_info.id))"},{"line_number":515,"context_line":"                    backup.update(state\u003dbackup_models.BackupState.FAILED)"},{"line_number":516,"context_line":"            task_api.API(self.context).delete_instance(self.id)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"        deltas \u003d {\u0027instances\u0027: -1}"}],"source_content_type":"text/x-python","patch_set":22,"id":"baada198_d6d59c1c","line":515,"in_reply_to":"baada198_02c1952e","updated":"2014-07-14 10:27:57.000000000","message":"Ok. Then this would be moved to BuiltInstanceTask.","commit_id":"45d4a615f154817b2cd15ea75552e659571ee7f1"}],"trove/tests/api/backups.py":[{"author":{"_account_id":9746,"name":"Doug Shelley","email":"dougshelley66@gmail.com","username":"dougshelley"},"change_message_id":"cad84488c3e57d1b558f0a041c5f4a423f206a02","unresolved":false,"context_lines":[{"line_number":462,"context_line":"    @test"},{"line_number":463,"context_line":"    def test_instance_created(self):"},{"line_number":464,"context_line":"        if CONFIG.fake_mode:"},{"line_number":465,"context_line":"            raise SkipTest(\"Should be executed not in real mode\")"},{"line_number":466,"context_line":"        self.instance_id \u003d RestoreUsingBackup._restore()"},{"line_number":467,"context_line":"        WaitForRestoreToFinish._poll(self.instance_id)"},{"line_number":468,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_668428b1","line":465,"updated":"2014-06-27 12:30:01.000000000","message":"Might be better worded as \"This is a fake mode test and shouldn\u0027t be executed in real mode\"\n\nHowever, I\u0027m trying to understand the \"if CONFIG.fake_mode\" wouldn\u0027t that make the test skip when in fake_mode? Can you clarify for me?","commit_id":"3961f1b7d2d4f386e6dd437091468cba739b509c"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"4220a7ca444dafe64828d6c4de1aefb0220b6892","unresolved":false,"context_lines":[{"line_number":455,"context_line":"        assert_raises(exceptions.NotFound, instance_info.dbaas.backups.get,"},{"line_number":456,"context_line":"                      incremental_info.id)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"@test(runs_after\u003d[DeleteBackups],"},{"line_number":460,"context_line":"      groups\u003d[GROUP, tests.INSTANCES])"},{"line_number":461,"context_line":"class TestFakeCreateFailedBackup(object):"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_25c6474a","line":458,"updated":"2014-07-08 23:15:12.000000000","message":"Instead of creating an int-test that only works in fake mode, wouldnt it be better to test this using unit tests? The way you are stubbing out a fake guest agent and overwriting its behaviour on the fly is confusing. It feels to me that you are trying to do an unit test in the form of a int-test","commit_id":"b73726aff0ba29775dc534cf54aec099b1240cd9"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"3c4b7baf6af9f6d36a051816f5f2b941a1f4675c","unresolved":false,"context_lines":[{"line_number":455,"context_line":"        assert_raises(exceptions.NotFound, instance_info.dbaas.backups.get,"},{"line_number":456,"context_line":"                      incremental_info.id)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"@test(runs_after\u003d[DeleteBackups],"},{"line_number":460,"context_line":"      groups\u003d[GROUP, tests.INSTANCES])"},{"line_number":461,"context_line":"class TestFakeCreateFailedBackup(object):"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_95ec762b","line":458,"in_reply_to":"baada198_0f5dd2e9","updated":"2014-07-09 23:19:23.000000000","message":"Deniz, you dont need a int-test to test this scenario. If we could it in real mode as well, i\u0027d be fine with it. I think this can be done as a unit test.\nTo create a unit test, you can mock Backup.list_for_instance to return you a backup mock, and then run the delete test. Then you can verify that the backup\u0027s status in the mock object is set to FAILED","commit_id":"b73726aff0ba29775dc534cf54aec099b1240cd9"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"b129a07485aa24000669b996ceab9e778a10e88c","unresolved":false,"context_lines":[{"line_number":455,"context_line":"        assert_raises(exceptions.NotFound, instance_info.dbaas.backups.get,"},{"line_number":456,"context_line":"                      incremental_info.id)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"@test(runs_after\u003d[DeleteBackups],"},{"line_number":460,"context_line":"      groups\u003d[GROUP, tests.INSTANCES])"},{"line_number":461,"context_line":"class TestFakeCreateFailedBackup(object):"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_0f5dd2e9","line":458,"in_reply_to":"baada198_25c6474a","updated":"2014-07-09 11:44:58.000000000","message":"Steve, this is an easiest way to test Trove\u0027s ReST framework. Also, there are tons for tests that are acting like unit tests.\n\nIn terms of Tempest tests terminology - this test is a scenarious test.\nIn terms of given issue, there\u0027s no way to implement unit test that will act like a scenarious test.","commit_id":"b73726aff0ba29775dc534cf54aec099b1240cd9"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"39ea7d3237de672406b30d3dae5971c2ad65a9ed","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                assert_not_equal(failed_status, backup.status)"},{"line_number":155,"context_line":"                return False"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        poll_until(result_is_active, time_out\u003dTIMEOUT_BACKUP_CREATE)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    @test"},{"line_number":160,"context_line":"    @time_out(TIMEOUT_BACKUP_CREATE)"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa98f980_b05b382d","line":157,"updated":"2014-09-01 21:34:36.000000000","message":"Why make all these changes when the basic logic of the tests have not changed? \n\nThe code seemed simpler (less LOC, and easier to read imo) before the rewrite, and the logic seems the same before and after the rewrite, so I\u0027d propose sticking with the code we already have.","commit_id":"6733dcefe3a9f70b0e6ce7564d07b82592a050f3"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"39ea7d3237de672406b30d3dae5971c2ad65a9ed","unresolved":false,"context_lines":[{"line_number":294,"context_line":"    def _restore(cls, backup_ref\u003dNone):"},{"line_number":295,"context_line":"        restore_point \u003d None"},{"line_number":296,"context_line":"        if backup_ref:"},{"line_number":297,"context_line":"            restore_point \u003d {\"backupRef\": backup_ref}"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        result \u003d instance_info.dbaas.instances.create("},{"line_number":300,"context_line":"            instance_info.name + \"_restore\","}],"source_content_type":"text/x-python","patch_set":31,"id":"fa98f980_50276cb6","line":297,"updated":"2014-09-01 21:34:36.000000000","message":"Seems irrelevant to the change being proposed.","commit_id":"6733dcefe3a9f70b0e6ce7564d07b82592a050f3"}]}