)]}'
{"etc/trove/trove-guestagent.conf.sample":[{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"64dee563a6470f6508450ae10d818556d95dede6","unresolved":false,"context_lines":[{"line_number":81,"context_line":"backup_use_gzip_compression \u003d True"},{"line_number":82,"context_line":"backup_use_openssl_encryption \u003d True"},{"line_number":83,"context_line":"backup_aes_cbc_key \u003d \"default_aes_cbc_key\""},{"line_number":84,"context_line":"backup_innobackupex_addl_options \u003d \"--no-lock\""},{"line_number":85,"context_line":"backup_use_snet \u003d False"},{"line_number":86,"context_line":"backup_chunk_size \u003d 65536"},{"line_number":87,"context_line":"backup_segment_max_size \u003d 2147483648"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAATn%2F%2Bj6s%3D","line":84,"updated":"2013-11-23 10:44:22.000000000","message":"Ane reasons to make it configurable ? Why not just use it where it needed without envolving config files ?","commit_id":"35bc36bc8111bfa05eaeb024d39c9c257c53ff01"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"c94fda787786682771347d234273b64ff2900a11","unresolved":false,"context_lines":[{"line_number":81,"context_line":"backup_use_gzip_compression \u003d True"},{"line_number":82,"context_line":"backup_use_openssl_encryption \u003d True"},{"line_number":83,"context_line":"backup_aes_cbc_key \u003d \"default_aes_cbc_key\""},{"line_number":84,"context_line":"backup_innobackupex_addl_options \u003d \"--no-lock\""},{"line_number":85,"context_line":"backup_use_snet \u003d False"},{"line_number":86,"context_line":"backup_chunk_size \u003d 65536"},{"line_number":87,"context_line":"backup_segment_max_size \u003d 2147483648"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAATn%2F%2Bduo%3D","line":84,"in_reply_to":"AAAATn%2F%2Bj6s%3D","updated":"2013-11-25 19:38:55.000000000","message":"You don\u0027t want to use this parameter, if you are using MyISAM tables, for example. \n\nMore generally, you want the list of parameters passed to innobackupex configurable, since each deployment might have a different way of creating the backup (maybe a different innobackupex binary version, or different tmpdir path, or whatever.)","commit_id":"35bc36bc8111bfa05eaeb024d39c9c257c53ff01"},{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"5d13a75df8347e72e56ff0eb172d54a9af9e6878","unresolved":false,"context_lines":[{"line_number":81,"context_line":"backup_use_gzip_compression \u003d True"},{"line_number":82,"context_line":"backup_use_openssl_encryption \u003d True"},{"line_number":83,"context_line":"backup_aes_cbc_key \u003d \"default_aes_cbc_key\""},{"line_number":84,"context_line":"backup_innobackupex_addl_options \u003d \"\""},{"line_number":85,"context_line":"backup_use_snet \u003d False"},{"line_number":86,"context_line":"backup_chunk_size \u003d 65536"},{"line_number":87,"context_line":"backup_segment_max_size \u003d 2147483648"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"AAAATn%2F%2BZUw%3D","line":84,"updated":"2013-11-26 15:22:50.000000000","message":"Comment out the line so that it can be an example:\n\n#backup_runner_options \u003d InnoBackupEx:--no-lock, MySQLDump:--events --routines --triggers","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"dcbc0b0bf0065e9da074c3f3fc16b02c5e88fba3","unresolved":false,"context_lines":[{"line_number":67,"context_line":"backup_use_gzip_compression \u003d True"},{"line_number":68,"context_line":"backup_use_openssl_encryption \u003d True"},{"line_number":69,"context_line":"backup_aes_cbc_key \u003d \"default_aes_cbc_key\""},{"line_number":70,"context_line":"backup_innobackupex_addl_options \u003d \"\""},{"line_number":71,"context_line":"backup_use_snet \u003d False"},{"line_number":72,"context_line":"backup_chunk_size \u003d 65536"},{"line_number":73,"context_line":"backup_segment_max_size \u003d 2147483648"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAATn%2F9%2F34%3D","line":70,"updated":"2013-12-03 16:43:31.000000000","message":"Please remove this line","commit_id":"85dd13a8ecf207c1f92f2d1b2a17232a1a271d75"}],"trove/common/cfg.py":[{"author":{"_account_id":1925,"name":"Vipul Sabhaya","email":"vipuls@gmail.com","username":"vipuls"},"change_message_id":"8ac5a479bb0c92b2751582e9389b6356f9e32e97","unresolved":false,"context_lines":[{"line_number":179,"context_line":"               help\u003d\u0027default openssl aes_cbc key.\u0027),"},{"line_number":180,"context_line":"    cfg.StrOpt(\u0027backup_innobackupex_addl_options\u0027,"},{"line_number":181,"context_line":"               default\u003d\u0027--no-lock\u0027,"},{"line_number":182,"context_line":"               help\u003d\u0027Additional options to pass to innobackupex\u0027),"},{"line_number":183,"context_line":"    cfg.BoolOpt(\u0027backup_use_snet\u0027, default\u003dFalse,"},{"line_number":184,"context_line":"                help\u003d\u0027Send backup files over snet.\u0027),"},{"line_number":185,"context_line":"    cfg.IntOpt(\u0027backup_chunk_size\u0027, default\u003d2 ** 16,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Bdsg%3D","line":182,"updated":"2013-11-25 19:41:08.000000000","message":"I don\u0027t recommend setting this as a default value for this.  The help text could be improved to specify how a parameter(s) would be added, but default should be None.","commit_id":"35bc36bc8111bfa05eaeb024d39c9c257c53ff01"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"cc814010edde9575efccf3da6c744cd1123bdc77","unresolved":false,"context_lines":[{"line_number":179,"context_line":"               help\u003d\u0027default openssl aes_cbc key.\u0027),"},{"line_number":180,"context_line":"    cfg.StrOpt(\u0027backup_innobackupex_addl_options\u0027,"},{"line_number":181,"context_line":"               default\u003d\u0027--no-lock\u0027,"},{"line_number":182,"context_line":"               help\u003d\u0027Additional options to pass to innobackupex\u0027),"},{"line_number":183,"context_line":"    cfg.BoolOpt(\u0027backup_use_snet\u0027, default\u003dFalse,"},{"line_number":184,"context_line":"                help\u003d\u0027Send backup files over snet.\u0027),"},{"line_number":185,"context_line":"    cfg.IntOpt(\u0027backup_chunk_size\u0027, default\u003d2 ** 16,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BdpA%3D","line":182,"in_reply_to":"AAAATn%2F%2Bdsg%3D","updated":"2013-11-25 19:51:27.000000000","message":"That\u0027s a fair suggestion.\nDone.","commit_id":"35bc36bc8111bfa05eaeb024d39c9c257c53ff01"},{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"5d13a75df8347e72e56ff0eb172d54a9af9e6878","unresolved":false,"context_lines":[{"line_number":177,"context_line":"                help\u003d\u0027Encrypt backups using openssl.\u0027),"},{"line_number":178,"context_line":"    cfg.StrOpt(\u0027backup_aes_cbc_key\u0027, default\u003d\u0027default_aes_cbc_key\u0027,"},{"line_number":179,"context_line":"               help\u003d\u0027default openssl aes_cbc key.\u0027),"},{"line_number":180,"context_line":"    cfg.StrOpt(\u0027backup_innobackupex_addl_options\u0027,"},{"line_number":181,"context_line":"               default\u003d\u0027\u0027,"},{"line_number":182,"context_line":"               help\u003d\u0027Additional options to pass to innobackupex\u0027),"},{"line_number":183,"context_line":"    cfg.BoolOpt(\u0027backup_use_snet\u0027, default\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BZUg%3D","line":180,"updated":"2013-11-26 15:22:50.000000000","message":"Change this to \u0027backup_runner_options\u0027 and make it a DictOpt with a default of {}","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"}],"trove/guestagent/strategies/backup/mysql_impl.py":[{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"5d13a75df8347e72e56ff0eb172d54a9af9e6878","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":24,"context_line":"CONF \u003d cfg.CONF"},{"line_number":25,"context_line":"BACKUP_INNOBACKUPEX_ADDL_OPTIONS \u003d CONF.backup_innobackupex_addl_options"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class MySQLDump(base.BackupRunner):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BZUY%3D","line":25,"updated":"2013-11-26 15:22:50.000000000","message":"this should be moved to backupagent and called \u0027BACKUP_RUNNER_OPTIONS\u0027","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"5d13a75df8347e72e56ff0eb172d54a9af9e6878","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    @property"},{"line_number":33,"context_line":"    def cmd(self):"},{"line_number":34,"context_line":"        cmd \u003d (\u0027/usr/bin/mysqldump\u0027"},{"line_number":35,"context_line":"               \u0027 --all-databases\u0027"},{"line_number":36,"context_line":"               \u0027 --opt\u0027"},{"line_number":37,"context_line":"               \u0027 --password\u003d%(password)s\u0027"},{"line_number":38,"context_line":"               \u0027 -u %(user)s\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BZV0%3D","line":35,"updated":"2013-11-26 15:22:50.000000000","message":"Add a line \u0027 %(extra_opts)s\u0027","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"5d13a75df8347e72e56ff0eb172d54a9af9e6878","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    @property"},{"line_number":52,"context_line":"    def cmd(self):"},{"line_number":53,"context_line":"        cmd \u003d (\u0027sudo innobackupex \u0027"},{"line_number":54,"context_line":"               \u0027--stream\u003dxbstream %s \u0027"},{"line_number":55,"context_line":"               \u0027/var/lib/mysql 2\u003e/tmp/innobackupex.log\u0027"},{"line_number":56,"context_line":"               ) % BACKUP_INNOBACKUPEX_ADDL_OPTIONS"},{"line_number":57,"context_line":"        return cmd + self.zip_cmd + self.encrypt_cmd"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BZWA%3D","line":54,"updated":"2013-11-26 15:22:50.000000000","message":"Add a new line \u0027 %(extra_opts)s\u0027 also I think it is better to keep the spaces before the next line as there is no need to change those lines in the review.","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"0b2de18f212adc8303c68544dd4a79d51877f55a","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        cmd \u003d (\u0027sudo innobackupex \u0027"},{"line_number":54,"context_line":"               \u0027--stream\u003dxbstream %s \u0027"},{"line_number":55,"context_line":"               \u0027/var/lib/mysql 2\u003e/tmp/innobackupex.log\u0027"},{"line_number":56,"context_line":"               ) % BACKUP_INNOBACKUPEX_ADDL_OPTIONS"},{"line_number":57,"context_line":"        return cmd + self.zip_cmd + self.encrypt_cmd"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BdLo%3D","line":56,"updated":"2013-11-25 21:15:48.000000000","message":"It seems to me that adding options like this will be a slippery slope. Each datastore and backup type will have extra options which will need config flags for. Why not just create a new class InnoBackupExNoLock which is a sub class of InnoBackupEx. Otherwise I think we need a better way to customize the backup command that is run.","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"7c78d24d09a0f13ae2640fdb6060434ff2aa9130","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        cmd \u003d (\u0027sudo innobackupex \u0027"},{"line_number":54,"context_line":"               \u0027--stream\u003dxbstream %s \u0027"},{"line_number":55,"context_line":"               \u0027/var/lib/mysql 2\u003e/tmp/innobackupex.log\u0027"},{"line_number":56,"context_line":"               ) % BACKUP_INNOBACKUPEX_ADDL_OPTIONS"},{"line_number":57,"context_line":"        return cmd + self.zip_cmd + self.encrypt_cmd"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BWko%3D","line":56,"in_reply_to":"AAAATn%2F%2BZUE%3D","updated":"2013-11-27 01:35:23.000000000","message":"This sounds like a good idea! Stay tuned for a new patchset implementing this. Thanks!","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":6268,"name":"Robert Myers","email":"myer0052@gmail.com","username":"rmyers"},"change_message_id":"5d13a75df8347e72e56ff0eb172d54a9af9e6878","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        cmd \u003d (\u0027sudo innobackupex \u0027"},{"line_number":54,"context_line":"               \u0027--stream\u003dxbstream %s \u0027"},{"line_number":55,"context_line":"               \u0027/var/lib/mysql 2\u003e/tmp/innobackupex.log\u0027"},{"line_number":56,"context_line":"               ) % BACKUP_INNOBACKUPEX_ADDL_OPTIONS"},{"line_number":57,"context_line":"        return cmd + self.zip_cmd + self.encrypt_cmd"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BZUE%3D","line":56,"in_reply_to":"AAAATn%2F%2BcNg%3D","updated":"2013-11-26 15:22:50.000000000","message":"Okay, then let\u0027s actually make this work for all backup types then. I already have a use case to add options for mysqldump.","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":1925,"name":"Vipul Sabhaya","email":"vipuls@gmail.com","username":"vipuls"},"change_message_id":"21c712ba43a408234cea646a8badf8bbb385462a","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        cmd \u003d (\u0027sudo innobackupex \u0027"},{"line_number":54,"context_line":"               \u0027--stream\u003dxbstream %s \u0027"},{"line_number":55,"context_line":"               \u0027/var/lib/mysql 2\u003e/tmp/innobackupex.log\u0027"},{"line_number":56,"context_line":"               ) % BACKUP_INNOBACKUPEX_ADDL_OPTIONS"},{"line_number":57,"context_line":"        return cmd + self.zip_cmd + self.encrypt_cmd"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2BcNg%3D","line":56,"in_reply_to":"AAAATn%2F%2Bcrk%3D","updated":"2013-11-26 01:41:06.000000000","message":"Agreed - I don\u0027t think it makes sense to implement several permutations of additional options that are available for a tool as different classes.","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"},{"author":{"_account_id":5293,"name":"Nikhil","email":"SlickNik@gmail.com","username":"slicknik"},"change_message_id":"862b97ecadfafbb37c2ef5ddae04b7c05ced0593","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        cmd \u003d (\u0027sudo innobackupex \u0027"},{"line_number":54,"context_line":"               \u0027--stream\u003dxbstream %s \u0027"},{"line_number":55,"context_line":"               \u0027/var/lib/mysql 2\u003e/tmp/innobackupex.log\u0027"},{"line_number":56,"context_line":"               ) % BACKUP_INNOBACKUPEX_ADDL_OPTIONS"},{"line_number":57,"context_line":"        return cmd + self.zip_cmd + self.encrypt_cmd"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2Bcrk%3D","line":56,"in_reply_to":"AAAATn%2F%2BdLo%3D","updated":"2013-11-25 23:03:36.000000000","message":"Agree with you that this will be a slippery slope if we have plans to include  *every* option that innobackupex supports. Trove should support only the bare-minimum, usable, default set of options that is necessary for backups to work correctly. This is what we should ship in the reference implementation, and this is what we should test in the gate. \n\n\nHowever, it should be up to each service provider what set of configuration flags they\u0027d like to support in their deployment. (Perhaps they want /tmp/ files to be written to a custom location, perhaps they want to support only innodb, and not MyISAM; I think that they should have the flexibility to tweak this according to their deployment).\n\n\nIn light of this, I\u0027ve updated this patch as well. I\u0027m not longer suggesting that we run with \"--no-lock\" enabled. What I\u0027m proposing instead, is that we have a setting in the config that enables service providers to use whatever config options makes most sense given their deployments.\n\n\nThanks!","commit_id":"98d902201e307a8ddbe133e9bfcb4af6ab370c1f"}]}