)]}'
{"trove/backup/models.py":[{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"7523060c070afafa0c87087ba2435b2727d5c68f","unresolved":false,"context_lines":[{"line_number":47,"context_line":"class Backup(object):"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    @classmethod"},{"line_number":50,"context_line":"    def validate_parent_backup_is_in_appropriate_state(cls, parent_backup):"},{"line_number":51,"context_line":"        if parent_backup.is_failed:"},{"line_number":52,"context_line":"            raise exception.ParentBackupFailedError("},{"line_number":53,"context_line":"                backup_id\u003dparent_backup.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_d909fd33","line":50,"updated":"2014-07-16 04:47:34.000000000","message":"No need to create a method to just check for a boolean. Why not just add those two lines in the caller block?","commit_id":"37bfb4307bae0a15300421ec51565484c5034008"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"b81174198498406b46d943fe6f1395aed22570c1","unresolved":false,"context_lines":[{"line_number":47,"context_line":"class Backup(object):"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    @classmethod"},{"line_number":50,"context_line":"    def validate_parent_backup_is_in_appropriate_state(cls, parent_backup):"},{"line_number":51,"context_line":"        if parent_backup.is_failed:"},{"line_number":52,"context_line":"            raise exception.ParentBackupFailedError("},{"line_number":53,"context_line":"                backup_id\u003dparent_backup.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_f45b23f4","line":50,"in_reply_to":"baada198_d909fd33","updated":"2014-07-16 08:40:05.000000000","message":"Agreed that this would probably be clearer inline. \nEspecially given the current naming -- what is an \"appropriate\" state for a backup? \n\nIf you inline the code, you can easily see that it\u0027s just checking for the \"Failed\" state.","commit_id":"37bfb4307bae0a15300421ec51565484c5034008"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"87ba25535540078dbcd2004d5373a7a4702f0496","unresolved":false,"context_lines":[{"line_number":84,"context_line":"            ds \u003d instance_model.datastore"},{"line_number":85,"context_line":"            ds_version \u003d instance_model.datastore_version"},{"line_number":86,"context_line":"            parent \u003d None"},{"line_number":87,"context_line":"            if parent_id:"},{"line_number":88,"context_line":"                # Look up the parent info or fail early if not found or if"},{"line_number":89,"context_line":"                # the user does not have access to the parent."},{"line_number":90,"context_line":"                _parent \u003d cls.get_by_id(context, parent_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_68944444","line":87,"updated":"2014-07-17 20:07:05.000000000","message":"Per discussion with Nikhil.\nWe need to move parent backup loading and validation before loading instance object to allow fast-failing if verified data was corrupted.","commit_id":"c889dc6b5b26ef75730643e91c14ec23e7fb583d"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"a83a996ee94953a24447809cc2e938cdb2634444","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                # Look up the parent info or fail early if not found or if"},{"line_number":89,"context_line":"                # the user does not have access to the parent."},{"line_number":90,"context_line":"                _parent \u003d cls.get_by_id(context, parent_id)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"                # Verifying that parent backup is not in FAILED state"},{"line_number":93,"context_line":"                # otherwise API service should raise BadRequest exception to"},{"line_number":94,"context_line":"                #  notify user that he\u0027s not permitted to do this operation"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_e6027971","line":91,"updated":"2014-07-16 20:13:46.000000000","message":"Hey Deniz, just nitpicking, but perhaps you dont need all these comments. I think comments are great when describing complicating logics but in this case it is pretty obvious what the code is trying to do. If the parent is bad, we dont create a child of it.\nLeaving comments for every bug fix leads to having the code hard to read and long. If you want to leave a comment, perhaps you can have a sentence. But i think it is ok with no comment.","commit_id":"c889dc6b5b26ef75730643e91c14ec23e7fb583d"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"25438d7e5c84a87403e316b9802210782e7f53cb","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        \"\"\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        def _create_resources():"},{"line_number":73,"context_line":"            # parse the ID from the Ref"},{"line_number":74,"context_line":"            parent \u003d None"},{"line_number":75,"context_line":"            if parent_id:"},{"line_number":76,"context_line":"                # Look up the parent info or fail early if not found or if"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_937d5e71","line":73,"updated":"2014-09-01 21:57:46.000000000","message":"You moved this code so that now this comment is not true anymore. Please fix this.","commit_id":"e2f65901759df17d0a235e7f3ae63e7903182668"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"105c1ddfed254240222fd041168550eafce4ddde","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        \"\"\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        def _create_resources():"},{"line_number":73,"context_line":"            # parse the ID from the Ref"},{"line_number":74,"context_line":"            parent \u003d None"},{"line_number":75,"context_line":"            if parent_id:"},{"line_number":76,"context_line":"                # Look up the parent info or fail early if not found or if"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_a3d749ec","line":73,"in_reply_to":"fa98f980_937d5e71","updated":"2014-09-02 15:06:25.000000000","message":"Done","commit_id":"e2f65901759df17d0a235e7f3ae63e7903182668"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"25438d7e5c84a87403e316b9802210782e7f53cb","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                    \u0027checksum\u0027: _parent.checksum,"},{"line_number":87,"context_line":"                }"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"            instance_id \u003d utils.get_id_from_href(instance)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"            # verify that the instance exists and can perform actions"},{"line_number":92,"context_line":"            from trove.instance.models import Instance"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_13916e99","line":89,"updated":"2014-09-01 21:57:46.000000000","message":"This is what the comment should refer to.","commit_id":"e2f65901759df17d0a235e7f3ae63e7903182668"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"105c1ddfed254240222fd041168550eafce4ddde","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                    \u0027checksum\u0027: _parent.checksum,"},{"line_number":87,"context_line":"                }"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"            instance_id \u003d utils.get_id_from_href(instance)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"            # verify that the instance exists and can perform actions"},{"line_number":92,"context_line":"            from trove.instance.models import Instance"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa98f980_c3052d7b","line":89,"in_reply_to":"fa98f980_13916e99","updated":"2014-09-02 15:06:25.000000000","message":"Done","commit_id":"e2f65901759df17d0a235e7f3ae63e7903182668"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"34c10439a19c1e7a9d47d75a93954de6d81b7dcd","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        \"\"\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        def _create_resources():"},{"line_number":73,"context_line":"            parent \u003d None"},{"line_number":74,"context_line":"            if parent_id:"},{"line_number":75,"context_line":"                # Look up the parent info or fail early if not found or if"},{"line_number":76,"context_line":"                # the user does not have access to the parent."}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_1e95c03f","line":73,"updated":"2014-09-10 20:10:59.000000000","message":"Why was this block of code moved up? Wouldn\u0027t it be more useful to quickly fail if a user cannot perform a backup, or has no access to swift?","commit_id":"21aae1c15050431b9fcdc1a701fc6cdb175f11f3"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"10562ddc7613e63dd6136b8d66d756cea71631b4","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        \"\"\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        def _create_resources():"},{"line_number":73,"context_line":"            parent \u003d None"},{"line_number":74,"context_line":"            if parent_id:"},{"line_number":75,"context_line":"                # Look up the parent info or fail early if not found or if"},{"line_number":76,"context_line":"                # the user does not have access to the parent."}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_bfb86376","line":73,"in_reply_to":"fa98f980_1e95c03f","updated":"2014-09-18 13:55:42.000000000","message":"This would allow fast-fail without performing tons of additional operations.","commit_id":"21aae1c15050431b9fcdc1a701fc6cdb175f11f3"}],"trove/common/exception.py":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"689a9541ed43c93e26ab65140c20f2c775798e9b","unresolved":false,"context_lines":[{"line_number":361,"context_line":""},{"line_number":362,"context_line":"class ParentBackupFailedError(TroveError):"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"    message \u003d _(\"Parent backup: %(backup_id)s is in FAILED state.\")"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"class BackupFileNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_822754c6","line":364,"updated":"2014-07-21 16:22:22.000000000","message":"please reword such as \"Cannot create incremental backup on a failed parent backup %(backup_id)s\" or some such.","commit_id":"d82e3a81877bce2b0bd0276231f506705793003e"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"34c10439a19c1e7a9d47d75a93954de6d81b7dcd","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"class ParentBackupFailedError(TroveError):"},{"line_number":369,"context_line":""},{"line_number":370,"context_line":"    message \u003d _(\"Parent backup: %(backup_id)s is in FAILED state.\")"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"class BackupFileNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_be8bd41c","line":370,"updated":"2014-09-10 20:10:59.000000000","message":"Parent backup failed would, I think suffice?","commit_id":"21aae1c15050431b9fcdc1a701fc6cdb175f11f3"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"10562ddc7613e63dd6136b8d66d756cea71631b4","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"class ParentBackupFailedError(TroveError):"},{"line_number":369,"context_line":""},{"line_number":370,"context_line":"    message \u003d _(\"Parent backup: %(backup_id)s is in FAILED state.\")"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"class BackupFileNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa98f980_9f6c7f0a","line":370,"in_reply_to":"fa98f980_be8bd41c","updated":"2014-09-18 13:55:42.000000000","message":"That may work too, but mine also looks good enough. Or there\u0027s something critical that i\u0027ve missed.","commit_id":"21aae1c15050431b9fcdc1a701fc6cdb175f11f3"}],"trove/common/wsgi.py":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"689a9541ed43c93e26ab65140c20f2c775798e9b","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        webob.exc.HTTPConflict: ["},{"line_number":339,"context_line":"            exception.BackupNotCompleteError,"},{"line_number":340,"context_line":"            exception.RestoreBackupIntegrityError,"},{"line_number":341,"context_line":"            exception.ParentBackupFailedError,"},{"line_number":342,"context_line":"        ],"},{"line_number":343,"context_line":"        webob.exc.HTTPRequestEntityTooLarge: ["},{"line_number":344,"context_line":"            exception.OverLimit,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_4208dc2c","line":341,"updated":"2014-07-21 16:22:22.000000000","message":"Why is this a conflict?","commit_id":"d82e3a81877bce2b0bd0276231f506705793003e"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"a2c387d5c4af768d7d6b08bf1e96411af64b4164","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        webob.exc.HTTPConflict: ["},{"line_number":339,"context_line":"            exception.BackupNotCompleteError,"},{"line_number":340,"context_line":"            exception.RestoreBackupIntegrityError,"},{"line_number":341,"context_line":"            exception.ParentBackupFailedError,"},{"line_number":342,"context_line":"        ],"},{"line_number":343,"context_line":"        webob.exc.HTTPRequestEntityTooLarge: ["},{"line_number":344,"context_line":"            exception.OverLimit,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_e83e318a","line":341,"in_reply_to":"baada198_4208dc2c","updated":"2014-07-21 16:46:20.000000000","message":"because used resource is not in valid state, see \nhttp://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html","commit_id":"d82e3a81877bce2b0bd0276231f506705793003e"}],"trove/tests/api/backups.py":[{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"b81174198498406b46d943fe6f1395aed22570c1","unresolved":false,"context_lines":[{"line_number":523,"context_line":"        assert_equal(400, instance_info.dbaas.last_http_code)"},{"line_number":524,"context_line":"        instance_info.dbaas.backups.delete(self.backup.id)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    @test(depends_on\u003d[test_restore_from_failed_backup])"},{"line_number":527,"context_line":"    def test_create_incrementail_backup_with_failed_parent(self):"},{"line_number":528,"context_line":"        if not CONFIG.fake_mode:"},{"line_number":529,"context_line":"            raise SkipTest(\"Only fake mode supported\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_c53f4b86","line":526,"updated":"2014-07-16 08:40:05.000000000","message":"This shouldn\u0027t depend on a restore test.","commit_id":"37bfb4307bae0a15300421ec51565484c5034008"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"b81174198498406b46d943fe6f1395aed22570c1","unresolved":false,"context_lines":[{"line_number":524,"context_line":"        instance_info.dbaas.backups.delete(self.backup.id)"},{"line_number":525,"context_line":""},{"line_number":526,"context_line":"    @test(depends_on\u003d[test_restore_from_failed_backup])"},{"line_number":527,"context_line":"    def test_create_incrementail_backup_with_failed_parent(self):"},{"line_number":528,"context_line":"        if not CONFIG.fake_mode:"},{"line_number":529,"context_line":"            raise SkipTest(\"Only fake mode supported\")"},{"line_number":530,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_e5236fa5","line":527,"updated":"2014-07-16 08:40:05.000000000","message":"incremental","commit_id":"37bfb4307bae0a15300421ec51565484c5034008"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"b81174198498406b46d943fe6f1395aed22570c1","unresolved":false,"context_lines":[{"line_number":528,"context_line":"        if not CONFIG.fake_mode:"},{"line_number":529,"context_line":"            raise SkipTest(\"Only fake mode supported\")"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"        self._create_and_verify()"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        backup \u003d instance_info.dbaas.backups.create("},{"line_number":534,"context_line":"            BACKUP_NAME, self.instance_id, BACKUP_DESC)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_e5cd2fb9","line":531,"updated":"2014-07-16 08:40:05.000000000","message":"Why not use the failed backup created above instead of creating a new one and patching it?","commit_id":"37bfb4307bae0a15300421ec51565484c5034008"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"f95957a4307303d5592ad372376454e965192cd7","unresolved":false,"context_lines":[{"line_number":528,"context_line":"        if not CONFIG.fake_mode:"},{"line_number":529,"context_line":"            raise SkipTest(\"Only fake mode supported\")"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"        self._create_and_verify()"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        backup \u003d instance_info.dbaas.backups.create("},{"line_number":534,"context_line":"            BACKUP_NAME, self.instance_id, BACKUP_DESC)"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_7491ccf0","line":531,"in_reply_to":"baada198_e5cd2fb9","updated":"2014-07-16 10:07:26.000000000","message":"See comments in next patchset","commit_id":"37bfb4307bae0a15300421ec51565484c5034008"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"08613f8d5e9aa8c0b3a026f55162cf86f6872501","unresolved":false,"context_lines":[{"line_number":531,"context_line":"        # We can\u0027t use backup from previous test because"},{"line_number":532,"context_line":"        # instance from which backup was taken was already deleted."},{"line_number":533,"context_line":"        # That\u0027s why we have to create new instance and"},{"line_number":534,"context_line":"        # take backup from it"},{"line_number":535,"context_line":"        self._create_and_verify()"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"        # Creating backup from fresh new instance"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_92b642f8","line":534,"updated":"2014-07-17 08:52:38.000000000","message":"It\u0027s totally possible to use the backup from the previous test - don\u0027t delete it in the last test, delete it in this one instead, and make this test dependent on test_create_failed_backup.","commit_id":"c889dc6b5b26ef75730643e91c14ec23e7fb583d"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"8a3b03c6eb6b5630bc6a5d3c74edee59dd158402","unresolved":false,"context_lines":[{"line_number":531,"context_line":"        # We can\u0027t use backup from previous test because"},{"line_number":532,"context_line":"        # instance from which backup was taken was already deleted."},{"line_number":533,"context_line":"        # That\u0027s why we have to create new instance and"},{"line_number":534,"context_line":"        # take backup from it"},{"line_number":535,"context_line":"        self._create_and_verify()"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"        # Creating backup from fresh new instance"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_19c7b265","line":534,"in_reply_to":"baada198_92b642f8","updated":"2014-07-17 11:52:33.000000000","message":"Disagree, If you  take a look at previous tests you will see that for the last tests we need two resources: instance and backup taken from this instance.\n\nTo create incremental backup instance should exists, but it was dropped at test_create_failed_backup (line #511).\nSo, when test_create_incremental_backup_with_failed_parent initialized we have only backup and no instance and we\u0027re not able to create incremental backup, because instance resource is missing.\n\nSo, without initial instance we cannot perform incremental backup test.\n\nThat\u0027s why we need to:\n\n1. create instance\n\n2. create failed backup\n\n3. verify that backup was created\n\n4. try to create incremental backup\n\nWithout two required resources - there\u0027s no way to perform required testing.\n\n\nPlease re-review it again.","commit_id":"c889dc6b5b26ef75730643e91c14ec23e7fb583d"}]}