)]}'
{"trove/backup/models.py":[{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"e5da59a951d91c79e4fa561336a25374a7f63484","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    @classmethod"},{"line_number":50,"context_line":"    def validate_can_create_incremental_backups(cls, instance, parent):"},{"line_number":51,"context_line":"        if not CONF.get(\u0027backup_incremental_strategy\u0027).get("},{"line_number":52,"context_line":"                parent.backup_type, None):"},{"line_number":53,"context_line":"            LOG.debug(\"Parent backup type: %s\" % parent.backup_type)"},{"line_number":54,"context_line":"            raise exception.DatastoreOperationNotSupported("},{"line_number":55,"context_line":"                operation\u003d\u0027incremental backup-create\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_0094d18a","line":52,"updated":"2014-07-16 08:54:47.000000000","message":"This is looking at the default CONF section, and not the specific datastore\u0027s CONF section. So based on this, how can you be sure that the datastore doesn\u0027t support this operation?","commit_id":"f6072876cef989ec8c766267c7db9e387fe97a06"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"3c1f207455ba9d4b330f2ce839949f9e916ddb6b","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    @classmethod"},{"line_number":50,"context_line":"    def validate_can_create_incremental_backups(cls, instance, parent):"},{"line_number":51,"context_line":"        if not CONF.get(\u0027backup_incremental_strategy\u0027).get("},{"line_number":52,"context_line":"                parent.backup_type, None):"},{"line_number":53,"context_line":"            LOG.debug(\"Parent backup type: %s\" % parent.backup_type)"},{"line_number":54,"context_line":"            raise exception.DatastoreOperationNotSupported("},{"line_number":55,"context_line":"                operation\u003d\u0027incremental backup-create\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_b6ae0b80","line":52,"in_reply_to":"baada198_0094d18a","updated":"2014-07-16 09:49:49.000000000","message":"As you know, backup_incremental_strategy is not a part of any datastore configuration group [1], so there\u0027s no need to use datastore configuration group.\nAnd even more, as you also know, each backup object contains backup type that refferes to datastore options group attribute - backup_strategy, so at the moment when user wants to create incremental backup from parent backup that has strategy that doesn\u0027t support incremental strategy(guest does it by looking at [2]) API service will look at the same option (backup_incremental_stretegy) and will discover if given backup (its strategy) have appropriate incremental strategy.\n\n\n\n[1] https://github.com/openstack/trove/blob/master/trove/common/cfg.py#L180-L184\n\n[2] https://github.com/openstack/trove/blob/master/trove/guestagent/backup/backupagent.py#L44-L45","commit_id":"f6072876cef989ec8c766267c7db9e387fe97a06"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"55285325cf2d388aac39cc9cf4535dc6ad9606d8","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    @classmethod"},{"line_number":50,"context_line":"    def validate_can_create_incremental_backups(cls, instance, parent):"},{"line_number":51,"context_line":"        \"\"\""},{"line_number":52,"context_line":"        This method does next things:"},{"line_number":53,"context_line":"         - retrieves parent backup \u0027backup_type\u0027 attribute that"},{"line_number":54,"context_line":"           correspondes to datastore options configuration"},{"line_number":55,"context_line":"           group \u0027backup_strategy\u0027."}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_cdeba2da","line":52,"updated":"2014-07-17 19:49:50.000000000","message":"Redundant.  Suggest \"Validates that the provided backup image supports incremental backups\"","commit_id":"e5b73c7322d670eccaf77e4fefc63cc575b2213a"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"55285325cf2d388aac39cc9cf4535dc6ad9606d8","unresolved":false,"context_lines":[{"line_number":62,"context_line":"           ~ if option exists (is not None) - validation passed;"},{"line_number":63,"context_line":"           ~ if ption doesn\u0027t exists (is None) - validation failed,"},{"line_number":64,"context_line":"             raising corresponding exception."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        @param instance: base instance model object"},{"line_number":67,"context_line":"        @type instance: BuiltInstance"},{"line_number":68,"context_line":"        @param parent: backup base model object"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_0d92fa4a","line":65,"updated":"2014-07-17 19:49:50.000000000","message":"The steps listed above are irrelevant to the caller (as they are implementation details), and do not provide any more information than the code (and in fact I had to read the code to understand the comments).  I suggest removing them from the doc string.","commit_id":"e5b73c7322d670eccaf77e4fefc63cc575b2213a"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"55285325cf2d388aac39cc9cf4535dc6ad9606d8","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                if _parent.is_failed:"},{"line_number":127,"context_line":"                    raise exception.ParentBackupFailedError("},{"line_number":128,"context_line":"                        backup_id\u003d_parent.id)"},{"line_number":129,"context_line":"                # Incremental backup feature validation"},{"line_number":130,"context_line":"                cls.validate_can_create_incremental_backups("},{"line_number":131,"context_line":"                    instance_model, _parent)"},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_eddce61a","line":129,"updated":"2014-07-17 19:49:50.000000000","message":"Irrelevant comment - the code is clear as to it\u0027s purpose as you have provided a meaningful name to the method.  Suggest removing the comment.","commit_id":"e5b73c7322d670eccaf77e4fefc63cc575b2213a"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"4728da78b4d862ac9047e70aa73096bd72e35ecd","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                    raise exception.ParentBackupFailedError("},{"line_number":97,"context_line":"                        backup_id\u003d_parent.id)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"                cls.validate_can_create_incremental_backups(_parent)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"                parent \u003d {"},{"line_number":102,"context_line":"                    \u0027location\u0027: _parent.location,"}],"source_content_type":"text/x-python","patch_set":15,"id":"7ac7a964_1c6079a4","line":99,"updated":"2014-08-16 00:36:43.000000000","message":"So, really, there is exactly one place in the code where we need to make this check. That is here.\n\nAnd for that, we could just as well check whether \n\nCONF.get(\u0027backup_incremental_strategy\u0027).get(parent.backup_type, None) is None.\n\nDo we really need a new function, a new exception, and a bunch of test cases?\n\nFine, if you want the test cases, so be it, but a new function and a new exception seem a tad bit excessive to me.","commit_id":"629f6ff46b122a1a034829eff5d6ed63f4d1ef08"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"8b58c3a7d4d1096bad8092a3ff4a917b93c0d503","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                    raise exception.ParentBackupFailedError("},{"line_number":97,"context_line":"                        backup_id\u003d_parent.id)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"                cls.validate_can_create_incremental_backups(_parent)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"                parent \u003d {"},{"line_number":102,"context_line":"                    \u0027location\u0027: _parent.location,"}],"source_content_type":"text/x-python","patch_set":15,"id":"7ac7a964_3d6fecfe","line":99,"in_reply_to":"7ac7a964_1c6079a4","updated":"2014-08-16 11:52:06.000000000","message":"About method.I followed the same pattern as we ded with backup strategies.\nAbout new exception. None of existing exceptions are fitting to this case (DatastoreOperationNotSupported doesn\u0027t work here, since it has nothing common with datastore, incremental backup strategy pinned to backup strategy, once incremental strategy would become a per datastore option, then we would be able to deprecate IncrementalBackupNotSupported exception)","commit_id":"629f6ff46b122a1a034829eff5d6ed63f4d1ef08"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"026108fd15641a57b088c6c141b436ebccf04c22","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        @param parent: backup base model object"},{"line_number":56,"context_line":"        @type parent: DBBackup"},{"line_number":57,"context_line":"        \"\"\""},{"line_number":58,"context_line":"        if not CONF.get(\u0027backup_incremental_strategy\u0027).get("},{"line_number":59,"context_line":"                parent.backup_type, None):"},{"line_number":60,"context_line":"            LOG.debug(\"Parent backup type: %s\" % parent.backup_type)"},{"line_number":61,"context_line":"            raise exception.IncrementalBackupNotSupported("}],"source_content_type":"text/x-python","patch_set":17,"id":"1abeadc6_9ab0c08c","line":58,"updated":"2014-08-28 18:03:47.000000000","message":"Amrith is making a change where this config is made datastore specific and not globally defined. https://review.openstack.org/#/c/113155/\n\nThis will not work once that is done. If this patch is merged first, you can let him know to make this change. If his patch is merged first, then you would have to change and specify a datastore.","commit_id":"60d223903c6313d6091f082ef99988055661c018"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"02de8dbfe69c903b4f401e3105e210b4845c9d18","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        @param parent: backup base model object"},{"line_number":56,"context_line":"        @type parent: DBBackup"},{"line_number":57,"context_line":"        \"\"\""},{"line_number":58,"context_line":"        if not CONF.get(\u0027backup_incremental_strategy\u0027).get("},{"line_number":59,"context_line":"                parent.backup_type, None):"},{"line_number":60,"context_line":"            LOG.debug(\"Parent backup type: %s\" % parent.backup_type)"},{"line_number":61,"context_line":"            raise exception.IncrementalBackupNotSupported("}],"source_content_type":"text/x-python","patch_set":17,"id":"1abeadc6_498e317a","line":58,"in_reply_to":"1abeadc6_786019a5","updated":"2014-08-29 17:47:52.000000000","message":"You are right. This should not deserve a -1.\nBut my -1 was because of the exception\u0027s message in the next file. Not because of this. I just wanted to let you know :)","commit_id":"60d223903c6313d6091f082ef99988055661c018"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"8bb849c0cd1716b59aa8cd8786a6e8d7836c3d8b","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        @param parent: backup base model object"},{"line_number":56,"context_line":"        @type parent: DBBackup"},{"line_number":57,"context_line":"        \"\"\""},{"line_number":58,"context_line":"        if not CONF.get(\u0027backup_incremental_strategy\u0027).get("},{"line_number":59,"context_line":"                parent.backup_type, None):"},{"line_number":60,"context_line":"            LOG.debug(\"Parent backup type: %s\" % parent.backup_type)"},{"line_number":61,"context_line":"            raise exception.IncrementalBackupNotSupported("}],"source_content_type":"text/x-python","patch_set":17,"id":"1abeadc6_786019a5","line":58,"in_reply_to":"1abeadc6_9ab0c08c","updated":"2014-08-29 16:20:19.000000000","message":"To be honest, this is not the reason for -1. We should not vote with -1 for something that wan\u0027t merged yet.","commit_id":"60d223903c6313d6091f082ef99988055661c018"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"80575eaebb5f22a82b43a4bb84c6039dc0a15462","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        Raises exception if backup incremental strategy is not supported"},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        datastore_cfg \u003d CONF.get(instance.datastore_version.manager)"},{"line_number":69,"context_line":"        if not datastore_cfg or not ("},{"line_number":70,"context_line":"                datastore_cfg.get("},{"line_number":71,"context_line":"                \u0027backup_incremental_strategy\u0027).get(parent.backup_type)):"},{"line_number":72,"context_line":"            raise exception.DatastoreOperationNotSupported("}],"source_content_type":"text/x-python","patch_set":21,"id":"fa98f980_be0834da","line":69,"updated":"2014-09-10 20:08:14.000000000","message":"splitting this in two would make it much easier to read.","commit_id":"d089f02e51a9b239304ade958c26b26ab1711cf8"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"9eafa260d6145eb526d3b0456cd9a734bbac7578","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        Raises exception if backup incremental strategy is not supported"},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        datastore_cfg \u003d CONF.get(instance.datastore_version.manager)"},{"line_number":69,"context_line":"        if not datastore_cfg or not ("},{"line_number":70,"context_line":"                datastore_cfg.get("},{"line_number":71,"context_line":"                \u0027backup_incremental_strategy\u0027).get(parent.backup_type)):"},{"line_number":72,"context_line":"            raise exception.DatastoreOperationNotSupported("}],"source_content_type":"text/x-python","patch_set":21,"id":"fa98f980_b16df7ce","line":69,"in_reply_to":"fa98f980_be0834da","updated":"2014-09-18 13:59:52.000000000","message":"Sure, i\u0027ll update it.","commit_id":"d089f02e51a9b239304ade958c26b26ab1711cf8"}],"trove/common/exception.py":[{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"026108fd15641a57b088c6c141b436ebccf04c22","unresolved":false,"context_lines":[{"line_number":152,"context_line":"class IncrementalBackupNotSupported(TroveError):"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    message \u003d _(\"Incremental backup feature is not \""},{"line_number":155,"context_line":"                \"implemented for %(strategy)s strategy.\")"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"class NoUniqueMatch(TroveError):"}],"source_content_type":"text/x-python","patch_set":17,"id":"1abeadc6_ba749ccf","line":155,"updated":"2014-08-28 18:03:47.000000000","message":"Since this error will go to the end user, the term of \u0027strategy\u0027 is unknown for this user. I suggest you to change this message to something like \"Incremental backup feature is not implemented for %(datastore)s datastore.\"","commit_id":"60d223903c6313d6091f082ef99988055661c018"}]}