)]}'
{"etc/trove/trove-guestagent.conf.sample":[{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"0207d55bf233a982b2895216d8382c690a793423","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"[mysql]"},{"line_number":74,"context_line":"backup_strategy \u003d InnoBackupEx"},{"line_number":75,"context_line":"[cassandra]"},{"line_number":76,"context_line":"backup_strategy \u003d NodetoolSnapshot"},{"line_number":77,"context_line":"[redis]"},{"line_number":78,"context_line":"backup_strategy \u003d NA"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAVX%2F%2F%2Bn8%3D","line":75,"updated":"2014-02-27 02:09:18.000000000","message":"1) using \"NA\" as an indicator of None is not correct. In cfg.py, you should define a cfg.OptGroup for redis and set the default for backup_strategy to None, then remove the [redis] subgroup here (because as far as this patch-set is concerned, there isn\u0027t anything you\u0027re setting).\n\n2) the cassandra backup implementation has not landed, therefore setting NodetoolSnapshot here is incorrect. As mentioned in the redis comment, remove the [cassandra] subgroup here and create an OptGroup in cfg.py with backup_strategy defualting to None.\n\n3) it would also make sense in this patch-set to move backup_namespace and restore_namespace as you did with backup_strategy (defaulting redis and cassandra to have None). That is, unless you\u0027re planning on renaming mysql_impl to something else, and have every single datastore\u0027s backup impl in one file?","commit_id":"f7cf71775709f6cd70ed906828e610e3935b024d"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"b2ef2a62772400ec7a1c5bb6ee2033d9b3cf49d1","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"[mysql]"},{"line_number":74,"context_line":"backup_strategy \u003d InnoBackupEx"},{"line_number":75,"context_line":"[cassandra]"},{"line_number":76,"context_line":"backup_strategy \u003d NodetoolSnapshot"},{"line_number":77,"context_line":"[redis]"},{"line_number":78,"context_line":"backup_strategy \u003d NA"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAVX%2F%2F%2BWI%3D","line":75,"in_reply_to":"AAAAVX%2F%2F%2Bn8%3D","updated":"2014-02-27 02:52:59.000000000","message":"amendment to my comment above: since backup_strategy is already clearly defined in cfg.py, and the likelihood that even a minority of users will write their own and desire an override is highly unlikely, let\u0027s not duplicate what\u0027s already in cfg.py here.\n\nin short, don\u0027t add anything to this file, instead keep it in cfg.py. then add an OptGroup for redis and cassandra in cfg.py with backup_strategy defaulting to None.","commit_id":"f7cf71775709f6cd70ed906828e610e3935b024d"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"f5f559387e9b4fe89f20bc0a41e1bd37e4c777a3","unresolved":false,"context_lines":[{"line_number":56,"context_line":"ignore_dbs \u003d lost+found, mysql, information_schema"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# Strategy information for backups"},{"line_number":59,"context_line":"backup_namespace \u003d trove.guestagent.strategies.backup.mysql_impl"},{"line_number":60,"context_line":"# Additional commandline options to be passed to the backup runner (by strategy). For example:"},{"line_number":61,"context_line":"# backup_runner_options \u003d InnoBackupEx:--no-lock,  MySQLDump:--events --routines --triggers"},{"line_number":62,"context_line":"restore_namespace \u003d trove.guestagent.strategies.restore.mysql_impl"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAVX%2F%2F3OE%3D","line":59,"updated":"2014-02-27 18:37:17.000000000","message":"as mentioned in patch-set 3:\n\nit would also make sense in this patch-set to move backup_namespace and restore_namespace as you did with backup_strategy (defaulting redis and cassandra to have None). That is, unless you\u0027re planning on renaming mysql_impl to something else, and have every single datastore\u0027s backup impl in one file?","commit_id":"bd04c531d8baf50ad487a6e9fdabb34fdb6571c5"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"cc10ee1ed58165d4e21778d4a4d1d0251fb6960c","unresolved":false,"context_lines":[{"line_number":56,"context_line":"ignore_dbs \u003d lost+found, mysql, information_schema"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# Strategy information for backups"},{"line_number":59,"context_line":"backup_namespace \u003d trove.guestagent.strategies.backup.mysql_impl"},{"line_number":60,"context_line":"# Additional commandline options to be passed to the backup runner (by strategy). For example:"},{"line_number":61,"context_line":"# backup_runner_options \u003d InnoBackupEx:--no-lock,  MySQLDump:--events --routines --triggers"},{"line_number":62,"context_line":"restore_namespace \u003d trove.guestagent.strategies.restore.mysql_impl"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAVX%2F%2F2PM%3D","line":59,"in_reply_to":"AAAAVX%2F%2F2Pw%3D","updated":"2014-02-27 21:20:39.000000000","message":"Yes, agreed, i\u0027ll do that as soon as this two patches would land.","commit_id":"bd04c531d8baf50ad487a6e9fdabb34fdb6571c5"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"b0e807f93e980b19c34a047dc470e74e556c06c2","unresolved":false,"context_lines":[{"line_number":56,"context_line":"ignore_dbs \u003d lost+found, mysql, information_schema"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# Strategy information for backups"},{"line_number":59,"context_line":"backup_namespace \u003d trove.guestagent.strategies.backup.mysql_impl"},{"line_number":60,"context_line":"# Additional commandline options to be passed to the backup runner (by strategy). For example:"},{"line_number":61,"context_line":"# backup_runner_options \u003d InnoBackupEx:--no-lock,  MySQLDump:--events --routines --triggers"},{"line_number":62,"context_line":"restore_namespace \u003d trove.guestagent.strategies.restore.mysql_impl"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAVX%2F%2F2Pw%3D","line":59,"in_reply_to":"AAAAVX%2F%2F2Q0%3D","updated":"2014-02-27 21:18:34.000000000","message":"ah, i see that now in https://review.openstack.org/#/c/73365/16/trove/guestagent/strategies/backup/impl.py. it\u0027s questionable as to whether all datastores should share a single impl for backups, but given the sheer # of outstanding interdependent code-reviews, i think this is fine to merge; we can always refactor them out later.","commit_id":"bd04c531d8baf50ad487a6e9fdabb34fdb6571c5"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"39f1c81932d716e36b5304f721c6526302860990","unresolved":false,"context_lines":[{"line_number":56,"context_line":"ignore_dbs \u003d lost+found, mysql, information_schema"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# Strategy information for backups"},{"line_number":59,"context_line":"backup_namespace \u003d trove.guestagent.strategies.backup.mysql_impl"},{"line_number":60,"context_line":"# Additional commandline options to be passed to the backup runner (by strategy). For example:"},{"line_number":61,"context_line":"# backup_runner_options \u003d InnoBackupEx:--no-lock,  MySQLDump:--events --routines --triggers"},{"line_number":62,"context_line":"restore_namespace \u003d trove.guestagent.strategies.restore.mysql_impl"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAVX%2F%2F2Q0%3D","line":59,"in_reply_to":"AAAAVX%2F%2F3OE%3D","updated":"2014-02-27 21:15:35.000000000","message":"Yes, i\u0027m planning to rename it into impl.py as it was long time ago.","commit_id":"bd04c531d8baf50ad487a6e9fdabb34fdb6571c5"}],"trove/common/cfg.py":[{"author":{"_account_id":8136,"name":"Viswa Vutharkar","email":"vpvutharkar@ebaysf.com","username":"vvutharkar"},"change_message_id":"033a28354336801c8a1bb08905d70b69f37da52e","unresolved":false,"context_lines":[{"line_number":269,"context_line":"mysql_group \u003d cfg.OptGroup("},{"line_number":270,"context_line":"    \u0027mysql\u0027, title\u003d\u0027MySQL options\u0027,"},{"line_number":271,"context_line":"    help\u003d\"Oslo option group designed for MySQL datastore\")"},{"line_number":272,"context_line":"mysql_opts \u003d ["},{"line_number":273,"context_line":"    cfg.StrOpt(\u0027backup_strategy\u0027, default\u003d\u0027InnoBackupEx\u0027,"},{"line_number":274,"context_line":"               help\u003d\u0027Default strategy to perform backups.\u0027),"},{"line_number":275,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2B8Q8%3D","line":272,"updated":"2014-02-18 03:30:23.000000000","message":"Denis, \nGlad you filed this bug and adding config groups to the guestagent.conf. I ran into exact same issues when implementing mongodb backup.\n\nAnother parameter to add is mount_point.  It is currently set to /var/lib/mysql and comes into picture at https://github.com/openstack/trove/blob/master/trove/guestagent/backup/backupagent.py#L86  .  The backup of my mongodb failed at this point because it failed to find /var/lib/mysql dir which won\u0027t exist on mongodb instance. This param needs to be pulled into the config groups as well and individually defined for each datastore type (/var/lib/mongodb  for [mongodb] group,  /var/lib/mysql for [mysql] group etc)","commit_id":"d2381fb9c8294cf6cc49c71a650e2420fbbe05f3"},{"author":{"_account_id":8136,"name":"Viswa Vutharkar","email":"vpvutharkar@ebaysf.com","username":"vvutharkar"},"change_message_id":"a80e00f4515b2851b098dd8c4b240f819a5732dc","unresolved":false,"context_lines":[{"line_number":269,"context_line":"mysql_group \u003d cfg.OptGroup("},{"line_number":270,"context_line":"    \u0027mysql\u0027, title\u003d\u0027MySQL options\u0027,"},{"line_number":271,"context_line":"    help\u003d\"Oslo option group designed for MySQL datastore\")"},{"line_number":272,"context_line":"mysql_opts \u003d ["},{"line_number":273,"context_line":"    cfg.StrOpt(\u0027backup_strategy\u0027, default\u003d\u0027InnoBackupEx\u0027,"},{"line_number":274,"context_line":"               help\u003d\u0027Default strategy to perform backups.\u0027),"},{"line_number":275,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2B0ws%3D","line":272,"in_reply_to":"AAAAUX%2F%2B4b4%3D","updated":"2014-02-18 19:32:22.000000000","message":"Great. Thx for the link.","commit_id":"d2381fb9c8294cf6cc49c71a650e2420fbbe05f3"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"3f8841ac9ec9ad107ce5a69af921d0389e553001","unresolved":false,"context_lines":[{"line_number":269,"context_line":"mysql_group \u003d cfg.OptGroup("},{"line_number":270,"context_line":"    \u0027mysql\u0027, title\u003d\u0027MySQL options\u0027,"},{"line_number":271,"context_line":"    help\u003d\"Oslo option group designed for MySQL datastore\")"},{"line_number":272,"context_line":"mysql_opts \u003d ["},{"line_number":273,"context_line":"    cfg.StrOpt(\u0027backup_strategy\u0027, default\u003d\u0027InnoBackupEx\u0027,"},{"line_number":274,"context_line":"               help\u003d\u0027Default strategy to perform backups.\u0027),"},{"line_number":275,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2B4b4%3D","line":272,"in_reply_to":"AAAAUX%2F%2B8Q8%3D","updated":"2014-02-18 12:54:08.000000000","message":"Here separate patch that fixes MP issues - https://review.openstack.org/#/c/74354/","commit_id":"d2381fb9c8294cf6cc49c71a650e2420fbbe05f3"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"9cbe5e9fbdab660aecf2894ca464c51792364c46","unresolved":false,"context_lines":[{"line_number":269,"context_line":"mysql_group \u003d cfg.OptGroup("},{"line_number":270,"context_line":"    \u0027mysql\u0027, title\u003d\u0027MySQL options\u0027,"},{"line_number":271,"context_line":"    help\u003d\"Oslo option group designed for MySQL datastore\")"},{"line_number":272,"context_line":"mysql_opts \u003d ["},{"line_number":273,"context_line":"    cfg.StrOpt(\u0027backup_strategy\u0027, default\u003d\u0027InnoBackupEx\u0027,"},{"line_number":274,"context_line":"               help\u003d\u0027Default strategy to perform backups.\u0027),"},{"line_number":275,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2B6vM%3D","line":272,"in_reply_to":"AAAAUX%2F%2B8Q8%3D","updated":"2014-02-18 08:00:09.000000000","message":"There\u0027s independent bug-report for mount points.\nBut yes, i agree, we already spoke about mount points issue. It would be make in a separate review, i suppose","commit_id":"d2381fb9c8294cf6cc49c71a650e2420fbbe05f3"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"0207d55bf233a982b2895216d8382c690a793423","unresolved":false,"context_lines":[{"line_number":268,"context_line":"# Mysql"},{"line_number":269,"context_line":"mysql_group \u003d cfg.OptGroup("},{"line_number":270,"context_line":"    \u0027mysql\u0027, title\u003d\u0027MySQL options\u0027,"},{"line_number":271,"context_line":"    help\u003d\"Oslo option group designed for MySQL datastore\")"},{"line_number":272,"context_line":"mysql_opts \u003d ["},{"line_number":273,"context_line":"    cfg.StrOpt(\u0027backup_strategy\u0027, default\u003d\u0027InnoBackupEx\u0027,"},{"line_number":274,"context_line":"               help\u003d\u0027Default strategy to perform backups.\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVX%2F%2F%2BlI%3D","line":271,"updated":"2014-02-27 02:09:18.000000000","message":"remove this help, it\u0027s not in https://review.openstack.org/#/c/50944/18/trove/common/cfg.py.\n\nalso, as mentioned in trove-taskmanager.conf.sample, OptGroups should exist here for Cassandra and Redis, each with the backup_strategy defaulted to None (and the subgroups for both removed from the conf).","commit_id":"f7cf71775709f6cd70ed906828e610e3935b024d"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"652d1b6b771e8cdd7130d5f078128df8b93b0fa3","unresolved":false,"context_lines":[{"line_number":268,"context_line":"# Mysql"},{"line_number":269,"context_line":"mysql_group \u003d cfg.OptGroup("},{"line_number":270,"context_line":"    \u0027mysql\u0027, title\u003d\u0027MySQL options\u0027,"},{"line_number":271,"context_line":"    help\u003d\"Oslo option group designed for MySQL datastore\")"},{"line_number":272,"context_line":"mysql_opts \u003d ["},{"line_number":273,"context_line":"    cfg.StrOpt(\u0027backup_strategy\u0027, default\u003d\u0027InnoBackupEx\u0027,"},{"line_number":274,"context_line":"               help\u003d\u0027Default strategy to perform backups.\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVX%2F%2F7b8%3D","line":271,"in_reply_to":"AAAAVX%2F%2F%2BlI%3D","updated":"2014-02-27 10:05:59.000000000","message":"I wouldn\u0027t remove it until https://review.openstack.org/#/c/50944/18/trove/common/cfg.py, then i\u0027ll just rebase the change. Thats all.","commit_id":"f7cf71775709f6cd70ed906828e610e3935b024d"}],"trove/guestagent/backup/backupagent.py":[{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"0207d55bf233a982b2895216d8382c690a793423","unresolved":false,"context_lines":[{"line_number":32,"context_line":"CONF \u003d cfg.CONF"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"MANAGER \u003d CONF.datastore_manager"},{"line_number":35,"context_line":"# Using mysql as defaul full-functioned datastore"},{"line_number":36,"context_line":"# manager that supports all API set"},{"line_number":37,"context_line":"STRATEGY \u003d CONF.get(\u0027mysql\u0027 if not MANAGER else MANAGER).backup_strategy"},{"line_number":38,"context_line":"NAMESPACE \u003d CONF.backup_namespace"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVX%2F%2F%2BkY%3D","line":35,"updated":"2014-02-27 02:09:18.000000000","message":"this comment is confusing. do you mean something like: \"Default to using mysql as the backup strategy in lieu of something being set in the conf\"?","commit_id":"f7cf71775709f6cd70ed906828e610e3935b024d"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"652d1b6b771e8cdd7130d5f078128df8b93b0fa3","unresolved":false,"context_lines":[{"line_number":32,"context_line":"CONF \u003d cfg.CONF"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"MANAGER \u003d CONF.datastore_manager"},{"line_number":35,"context_line":"# Using mysql as defaul full-functioned datastore"},{"line_number":36,"context_line":"# manager that supports all API set"},{"line_number":37,"context_line":"STRATEGY \u003d CONF.get(\u0027mysql\u0027 if not MANAGER else MANAGER).backup_strategy"},{"line_number":38,"context_line":"NAMESPACE \u003d CONF.backup_namespace"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAVX%2F%2F7aw%3D","line":35,"in_reply_to":"AAAAVX%2F%2F%2BkY%3D","updated":"2014-02-27 10:05:59.000000000","message":"I mean that \"mysql\" would be used if manager is not mentioned in config files at all","commit_id":"f7cf71775709f6cd70ed906828e610e3935b024d"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"f5f559387e9b4fe89f20bc0a41e1bd37e4c777a3","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"MANAGER \u003d CONF.datastore_manager"},{"line_number":35,"context_line":"# If datastore manager is not mentioned in guest"},{"line_number":36,"context_line":"# configuration file, would be used mysql as datastore_manager by the default"},{"line_number":37,"context_line":"STRATEGY \u003d CONF.get(\u0027mysql\u0027 if not MANAGER else MANAGER).backup_strategy"},{"line_number":38,"context_line":"NAMESPACE \u003d CONF.backup_namespace"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVX%2F%2F3Ng%3D","line":36,"updated":"2014-02-27 18:37:17.000000000","message":"a slight grammar fix:\n\n\"would be used mysql as datastore_manager by the default\"\n\nshould be:\n\n\"mysql is used as the datastore_manager by default\"","commit_id":"bd04c531d8baf50ad487a6e9fdabb34fdb6571c5"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"6079811b1d19d572192524ab4154c3f307e734cc","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"MANAGER \u003d CONF.datastore_manager"},{"line_number":35,"context_line":"# If datastore manager is not mentioned in guest"},{"line_number":36,"context_line":"# configuration file, would be used mysql as datastore_manager by the default"},{"line_number":37,"context_line":"STRATEGY \u003d CONF.get(\u0027mysql\u0027 if not MANAGER else MANAGER).backup_strategy"},{"line_number":38,"context_line":"NAMESPACE \u003d CONF.backup_namespace"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVX%2F%2F1%2Bo%3D","line":36,"in_reply_to":"AAAAVX%2F%2F2Qw%3D","updated":"2014-02-27 22:04:24.000000000","message":"there are tons of interdependent reviews, and denis can\u0027t change this until a day or so from now, so this can be fixed later.","commit_id":"bd04c531d8baf50ad487a6e9fdabb34fdb6571c5"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"39f1c81932d716e36b5304f721c6526302860990","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"MANAGER \u003d CONF.datastore_manager"},{"line_number":35,"context_line":"# If datastore manager is not mentioned in guest"},{"line_number":36,"context_line":"# configuration file, would be used mysql as datastore_manager by the default"},{"line_number":37,"context_line":"STRATEGY \u003d CONF.get(\u0027mysql\u0027 if not MANAGER else MANAGER).backup_strategy"},{"line_number":38,"context_line":"NAMESPACE \u003d CONF.backup_namespace"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVX%2F%2F2Qw%3D","line":36,"in_reply_to":"AAAAVX%2F%2F3Ng%3D","updated":"2014-02-27 21:15:35.000000000","message":"Thanks, get it.","commit_id":"bd04c531d8baf50ad487a6e9fdabb34fdb6571c5"}]}