)]}'
{"trove/common/cfg.py":[{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"348c9ff7d3d3271afb917f822cd4652af9bcffe5","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                \u0027the root user is immediately returned in the response of \u0027"},{"line_number":138,"context_line":"                \"instance-create as the \u0027password\u0027 field.\"),"},{"line_number":139,"context_line":"    cfg.StrOpt(\u0027mount_point\u0027, default\u003d\u0027/var/lib/datastore\u0027,"},{"line_number":140,"context_line":"               help\u003d\u0027fallback datastore mount location\u0027),"},{"line_number":141,"context_line":"    cfg.ListOpt(\u0027root_grant\u0027, default\u003d[\u0027ALL\u0027]),"},{"line_number":142,"context_line":"    cfg.BoolOpt(\u0027root_grant_option\u0027, default\u003dTrue),"},{"line_number":143,"context_line":"    cfg.IntOpt(\u0027default_password_length\u0027, default\u003d36),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FIT4%3D","line":140,"updated":"2014-03-08 16:36:13.000000000","message":"Help string should be matching to the common help for mount_point parameter for defined parameters.","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"fe8a8558be1c7d1ee4e5663e6f50a017c8fd1307","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                \u0027the root user is immediately returned in the response of \u0027"},{"line_number":138,"context_line":"                \"instance-create as the \u0027password\u0027 field.\"),"},{"line_number":139,"context_line":"    cfg.StrOpt(\u0027mount_point\u0027, default\u003d\u0027/var/lib/datastore\u0027,"},{"line_number":140,"context_line":"               help\u003d\u0027fallback datastore mount location\u0027),"},{"line_number":141,"context_line":"    cfg.ListOpt(\u0027root_grant\u0027, default\u003d[\u0027ALL\u0027]),"},{"line_number":142,"context_line":"    cfg.BoolOpt(\u0027root_grant_option\u0027, default\u003dTrue),"},{"line_number":143,"context_line":"    cfg.IntOpt(\u0027default_password_length\u0027, default\u003d36),"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FFd4%3D","line":140,"in_reply_to":"AAAAVn%2F%2FIT4%3D","updated":"2014-03-10 02:27:42.000000000","message":"disagree with adding this back, see my comment in models.py","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"}],"trove/taskmanager/models.py":[{"author":{"_account_id":6156,"name":"Justin Hopper","email":"justin.hopper@hp.com","username":"justin-hopper"},"change_message_id":"f8e74e8045330088e3225eb5a8290e36767e0ca9","unresolved":false,"context_lines":[{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        device_path \u003d CONF.device_path"},{"line_number":444,"context_line":"        mount_point \u003d CONF.get(\u0027mysql\u0027 if not CONF.datastore_manager"},{"line_number":445,"context_line":"                               else CONF.datastore_manager).mount_point"},{"line_number":446,"context_line":"        volume_info \u003d {\u0027device_path\u0027: device_path, \u0027mount_point\u0027: mount_point}"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"        LOG.debug(_(\"end _create_server_volume_heat for id: %s\") % self.id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2FUEo%3D","line":445,"updated":"2014-03-06 21:56:27.000000000","message":"This goes back to a recent review where it was stated the pythonic thing to do is to surround with try/except.  where the exception could log a warning and set it to a reasonable default.  \n\nIf there is no reasonable default (and I don\u0027t think mysql is the answer) AND this value must be set, then throw the exception.  In this case it is not an option but a required value that someone did not specify.","commit_id":"d5468caeebb05f4bdfe380c3a8c1391487472e59"},{"author":{"_account_id":6162,"name":"Dan Nguyen","email":"dan.nguyens.mail@gmail.com","username":"daniel-a-nguyen"},"change_message_id":"252515bcdca06128fe60d95b2436e49f54d76241","unresolved":false,"context_lines":[{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        device_path \u003d CONF.device_path"},{"line_number":444,"context_line":"        mount_point \u003d CONF.get(\u0027mysql\u0027 if not CONF.datastore_manager"},{"line_number":445,"context_line":"                               else CONF.datastore_manager).mount_point"},{"line_number":446,"context_line":"        volume_info \u003d {\u0027device_path\u0027: device_path, \u0027mount_point\u0027: mount_point}"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"        LOG.debug(_(\"end _create_server_volume_heat for id: %s\") % self.id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2FVDE%3D","line":445,"updated":"2014-03-06 19:48:59.000000000","message":"This is a good catch but I\u0027m not 100% certain we can fall back on mysql as the default datastore + mount point.\n\nI\u0027m thinking of 3 possible ways to get around this.\n\n1) have a default mount_point that is not specific to a particular datastore  (i.e. /var/lib/datastore)\n\n2) don\u0027t include the mount point in the volume_info\n\n3) make every datastore type define the mount_point and update the trove-taskmanger.conf to set the datastore_manager so that it is not None.\n\nI\u0027m leaning towards 3 and I think we could update the bug to include requiring these datastore properties in the sample config files that will use them (trove-taskmanager.conf.sample).","commit_id":"d5468caeebb05f4bdfe380c3a8c1391487472e59"},{"author":{"_account_id":8311,"name":"SnowDust","email":"nilakhya.chatterjee@globallogic.com","username":"nilc"},"change_message_id":"40bc55ba39535c933b7e7f32a33c20aae28d75cd","unresolved":false,"context_lines":[{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        device_path \u003d CONF.device_path"},{"line_number":444,"context_line":"        mount_point \u003d CONF.get(\u0027mysql\u0027 if not CONF.datastore_manager"},{"line_number":445,"context_line":"                               else CONF.datastore_manager).mount_point"},{"line_number":446,"context_line":"        volume_info \u003d {\u0027device_path\u0027: device_path, \u0027mount_point\u0027: mount_point}"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"        LOG.debug(_(\"end _create_server_volume_heat for id: %s\") % self.id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2FRbA%3D","line":445,"in_reply_to":"AAAAVn%2F%2FUEo%3D","updated":"2014-03-07 05:39:42.000000000","message":"thanks justin, dan,\n\nAgreed,\n\nThe solution is the approach where we re-introduce \u0027mount_point\u0027 in the default section,  and just as justin said to have a try/except to fall back on it ( dan i think the same is your approach 1)","commit_id":"d5468caeebb05f4bdfe380c3a8c1391487472e59"},{"author":{"_account_id":6162,"name":"Dan Nguyen","email":"dan.nguyens.mail@gmail.com","username":"daniel-a-nguyen"},"change_message_id":"857691fdfa5f77ab49d5449a8d0c116f5e81023e","unresolved":false,"context_lines":[{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        device_path \u003d CONF.device_path"},{"line_number":444,"context_line":"        mount_point \u003d CONF.get(\u0027mysql\u0027 if not CONF.datastore_manager"},{"line_number":445,"context_line":"                               else CONF.datastore_manager).mount_point"},{"line_number":446,"context_line":"        volume_info \u003d {\u0027device_path\u0027: device_path, \u0027mount_point\u0027: mount_point}"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"        LOG.debug(_(\"end _create_server_volume_heat for id: %s\") % self.id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2FUP4%3D","line":445,"in_reply_to":"AAAAVn%2F%2FUy4%3D","updated":"2014-03-06 21:43:46.000000000","message":"hi nilakhya,\n\nI think this is a great bug but I feel like the fix above makes the problem go away instead of addressing the underlying issue.\n\nIf we expect properties to be present in our code we should make sure they are there or have reasonable defaults.  I\u0027m not sure mysql is a reasonable default because if you have a mongo datastore and you set the mount_point to /var/lib/mysql it does\u0027t make a lot of sense. \n\nSo your patch could just be updating the trove-taskmanager.conf.sample and you would update your real config file to have the datastore_manager and mount_point property.\n\nIf you have an experimental datastore you would want to make sure that you have something configured for the mount_point.\n\nI could me missing something here so let me know if I got it wrong.","commit_id":"d5468caeebb05f4bdfe380c3a8c1391487472e59"},{"author":{"_account_id":8311,"name":"SnowDust","email":"nilakhya.chatterjee@globallogic.com","username":"nilc"},"change_message_id":"9cffd8f74fb22422cdabab72388ba56101eda396","unresolved":false,"context_lines":[{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        device_path \u003d CONF.device_path"},{"line_number":444,"context_line":"        mount_point \u003d CONF.get(\u0027mysql\u0027 if not CONF.datastore_manager"},{"line_number":445,"context_line":"                               else CONF.datastore_manager).mount_point"},{"line_number":446,"context_line":"        volume_info \u003d {\u0027device_path\u0027: device_path, \u0027mount_point\u0027: mount_point}"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"        LOG.debug(_(\"end _create_server_volume_heat for id: %s\") % self.id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2FUy4%3D","line":445,"in_reply_to":"AAAAVn%2F%2FVDE%3D","updated":"2014-03-06 20:30:06.000000000","message":"dan,\n\napproach 1) is acceptable and i have no problems with it. The present patch was in favor of similar code in backupagent.py already approved and merged.\n\nalso approach 1) will bring in  again cfg.StrOpt(\u0027mount_point\u0027  ... \n\n2) is a design change and i have no purpose to change it ;)\n\n3) is not acceptable and defeats the purpose of this patch. Say i have an eperimental datastore, for which i have my separate manager class dynamically loaded beautifully.  \n\nbut now taskmanager screwed by cfg.py hard bindings of CONF.datastore_manager not registered there for the experimental datastore.\n\ni have to write extra lines in cfg.py  similar to: \nexperimental_opts \u003d .......\nCONF.register_group(experimental_group)\nCONF.register_opts(experimental_opts, experimental_group) this is not anymore dynamic, a jailed datastore manager.","commit_id":"d5468caeebb05f4bdfe380c3a8c1391487472e59"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"1327d8081f37b3d773850efde5f6dc8bbcb792ed","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    try:"},{"line_number":79,"context_line":"        mount_point \u003d CONF.get(datastore_manager).mount_point"},{"line_number":80,"context_line":"    except NoSuchOptError:"},{"line_number":81,"context_line":"        mount_point \u003d CONF.mount_point"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"class NotifyMixin(object):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FPKg%3D","line":81,"updated":"2014-03-07 11:39:45.000000000","message":"We are not returning any values from function, which seems to be used whenever get_mount_point is called.","commit_id":"24a35934b75bafdb2306567abdae4b8a93640018"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"1327d8081f37b3d773850efde5f6dc8bbcb792ed","unresolved":false,"context_lines":[{"line_number":379,"context_line":"            self._log_and_raise(e, msg, err)"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        device_path \u003d CONF.device_path"},{"line_number":382,"context_line":"        mount_point \u003d get_mount_point(datastore_manager)"},{"line_number":383,"context_line":"        volume_info \u003d {\u0027device_path\u0027: device_path, \u0027mount_point\u0027: mount_point}"},{"line_number":384,"context_line":"        LOG.debug(_(\"end _create_server_volume for id: %s\") % self.id)"},{"line_number":385,"context_line":"        return volume_info"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2FPKc%3D","line":382,"updated":"2014-03-07 11:39:45.000000000","message":"I think we would be needing the additional unit tests around mount_point assignment using this method, because even though the mount_point was assigned a null value, still python26 and python27 tests passed without any failure.","commit_id":"24a35934b75bafdb2306567abdae4b8a93640018"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"348c9ff7d3d3271afb917f822cd4652af9bcffe5","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from eventlet import greenthread"},{"line_number":22,"context_line":"from novaclient import exceptions as nova_exceptions"},{"line_number":23,"context_line":"from swiftclient.client import ClientException"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"from oslo.config.cfg import NoSuchOptError"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from trove.backup import models as bkup_models"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FISo%3D","line":24,"updated":"2014-03-08 16:36:13.000000000","message":"not sure why do we have blank line","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"348c9ff7d3d3271afb917f822cd4652af9bcffe5","unresolved":false,"context_lines":[{"line_number":74,"context_line":"use_heat \u003d CONF.use_heat"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"# get a fallback mount_point or use the preferred if available."},{"line_number":78,"context_line":"def get_mount_point(datastore_manager):"},{"line_number":79,"context_line":"    try:"},{"line_number":80,"context_line":"        mount_point \u003d CONF.get(datastore_manager).mount_point"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FISA%3D","line":77,"updated":"2014-03-08 16:36:13.000000000","message":"Not sure about the purpose of this comment, rather it would be better to provide the a docstring for the method.","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"},{"author":{"_account_id":6156,"name":"Justin Hopper","email":"justin.hopper@hp.com","username":"justin-hopper"},"change_message_id":"75be6444f104a7ceaaa7ed0fed942be0eb6eac57","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"# get a fallback mount_point or use the preferred if available."},{"line_number":78,"context_line":"def get_mount_point(datastore_manager):"},{"line_number":79,"context_line":"    try:"},{"line_number":80,"context_line":"        mount_point \u003d CONF.get(datastore_manager).mount_point"},{"line_number":81,"context_line":"    except NoSuchOptError:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FGgE%3D","line":78,"updated":"2014-03-09 19:45:10.000000000","message":"I would change this to a lazy initialized attribute of NotifyMixin where the the attribute __mount_point is initialized once and there is a guard statement to do the initialization once...e.g. if not self.__mount_point: try .... return self.__mount_point.  This way the exception is triggered once in the case...not for every invocation of get_mount_point()","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"},{"author":{"_account_id":8214,"name":"Auston McReynolds","email":"amcreynolds@ebaysf.com","username":"amcreynolds"},"change_message_id":"fe8a8558be1c7d1ee4e5663e6f50a017c8fd1307","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"# get a fallback mount_point or use the preferred if available."},{"line_number":78,"context_line":"def get_mount_point(datastore_manager):"},{"line_number":79,"context_line":"    try:"},{"line_number":80,"context_line":"        mount_point \u003d CONF.get(datastore_manager).mount_point"},{"line_number":81,"context_line":"    except NoSuchOptError:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FFdo%3D","line":78,"in_reply_to":"AAAAVn%2F%2FGgE%3D","updated":"2014-03-10 02:27:42.000000000","message":"although that would work, the established pattern thus far is seen at https://github.com/openstack/trove/blob/master/trove/guestagent/backup/backupagent.py#L89-L90\n\n    mount_point \u003d CONF.get(\u0027mysql\u0027 if not CONF.datastore_manager\n                           else CONF.datastore_manager).mount_point\n\nwith https://github.com/openstack/trove/commit/ea51b7e625b6be4415a665cd36dab44742b0362e, unless the operator has overridden the [mysql] mount_point to something nonsensical, then the \"mysql if not\" logic should suffice.","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"348c9ff7d3d3271afb917f822cd4652af9bcffe5","unresolved":false,"context_lines":[{"line_number":77,"context_line":"# get a fallback mount_point or use the preferred if available."},{"line_number":78,"context_line":"def get_mount_point(datastore_manager):"},{"line_number":79,"context_line":"    try:"},{"line_number":80,"context_line":"        mount_point \u003d CONF.get(datastore_manager).mount_point"},{"line_number":81,"context_line":"    except NoSuchOptError:"},{"line_number":82,"context_line":"        mount_point \u003d CONF.mount_point"},{"line_number":83,"context_line":"    return mount_point"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FIT0%3D","line":80,"updated":"2014-03-08 16:36:13.000000000","message":"can we not use \n\nreturn CONF.get(datastore_manager).mount_point","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"348c9ff7d3d3271afb917f822cd4652af9bcffe5","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    try:"},{"line_number":80,"context_line":"        mount_point \u003d CONF.get(datastore_manager).mount_point"},{"line_number":81,"context_line":"    except NoSuchOptError:"},{"line_number":82,"context_line":"        mount_point \u003d CONF.mount_point"},{"line_number":83,"context_line":"    return mount_point"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FITw%3D","line":82,"updated":"2014-03-08 16:36:13.000000000","message":"can we not use \n\nreturn CONF.mount_point","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"},{"author":{"_account_id":7806,"name":"Sushil Kumar","email":"skm.net@gmail.com","username":"sushilkm"},"change_message_id":"348c9ff7d3d3271afb917f822cd4652af9bcffe5","unresolved":false,"context_lines":[{"line_number":381,"context_line":"            self._log_and_raise(e, msg, err)"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        device_path \u003d CONF.device_path"},{"line_number":384,"context_line":"        mount_point \u003d get_mount_point(datastore_manager)"},{"line_number":385,"context_line":"        volume_info \u003d {\u0027device_path\u0027: device_path, \u0027mount_point\u0027: mount_point}"},{"line_number":386,"context_line":"        LOG.debug(_(\"end _create_server_volume for id: %s\") % self.id)"},{"line_number":387,"context_line":"        return volume_info"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAVn%2F%2FIS0%3D","line":384,"updated":"2014-03-08 16:36:13.000000000","message":"I think we should also have some tests around the way we are providing value to mount_point as in one of the earlier patchsets used, python26 and python27 tests have passed even when there was no return statement inside the \"get_mount_point\" method defined.","commit_id":"f6f827923acdc1ca4657dcac5de17534f646b9c2"}]}