)]}'
{"cinder/cmd/status.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7496bc8fca77f1a28fb22f2a71c5806b7a2a51ec","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                \u0027the driver, but current setting is using only the module \u0027"},{"line_number":52,"context_line":"                \u0027path.\u0027)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        return uc.Result(SUCCESS, \u0027Some details\u0027)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    _upgrade_checks \u003d ("},{"line_number":57,"context_line":"        (\u0027Backup Driver Path\u0027, _check_backup_module),"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_fb095f2a","line":54,"range":{"start_line":54,"start_character":34,"end_line":54,"end_character":48},"updated":"2019-03-26 21:52:51.000000000","message":"Fixed in a follow up patch.","commit_id":"b886d93fc197df6d4135ebf89bdc494f8a09aaa5"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"d8db4cbdbf10f054f697591738bee6e0a5dc9140","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        if backup_driver \u003d\u003d backup_driver.lower():"},{"line_number":48,"context_line":"            return uc.Result("},{"line_number":49,"context_line":"                FAILURE,"},{"line_number":50,"context_line":"                \u0027Backup driver configuration requires the full path to the \u0027"},{"line_number":51,"context_line":"                \u0027the driver, but current setting is using only the module \u0027"},{"line_number":52,"context_line":"                \u0027path.\u0027)"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_cec83b23","line":50,"range":{"start_line":50,"start_character":71,"end_line":50,"end_character":75},"updated":"2019-03-28 12:29:37.000000000","message":"the the","commit_id":"ae99b139bfea19050ebbb44b38755b514f13b817"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4ef37d4531cac6b218a1529d63502d1a9d8ff384","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        if backup_driver \u003d\u003d backup_driver.lower():"},{"line_number":48,"context_line":"            return uc.Result("},{"line_number":49,"context_line":"                FAILURE,"},{"line_number":50,"context_line":"                \u0027Backup driver configuration requires the full path to the \u0027"},{"line_number":51,"context_line":"                \u0027the driver, but current setting is using only the module \u0027"},{"line_number":52,"context_line":"                \u0027path.\u0027)"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_65b3650b","line":50,"range":{"start_line":50,"start_character":71,"end_line":50,"end_character":75},"in_reply_to":"5fc1f717_423dffb8","updated":"2019-03-28 13:44:31.000000000","message":"Wow, I\u0027ve reread what I wrote several times and still keep missing little issues. Update coming.","commit_id":"ae99b139bfea19050ebbb44b38755b514f13b817"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"974af57f043400aaa5949ba03a20c8aa0f7f3390","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        if backup_driver \u003d\u003d backup_driver.lower():"},{"line_number":48,"context_line":"            return uc.Result("},{"line_number":49,"context_line":"                FAILURE,"},{"line_number":50,"context_line":"                \u0027Backup driver configuration requires the full path to the \u0027"},{"line_number":51,"context_line":"                \u0027the driver, but current setting is using only the module \u0027"},{"line_number":52,"context_line":"                \u0027path.\u0027)"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_423dffb8","line":50,"range":{"start_line":50,"start_character":71,"end_line":50,"end_character":75},"in_reply_to":"5fc1f717_cec83b23","updated":"2019-03-28 12:50:44.000000000","message":"Good eye! I completely missed that one.","commit_id":"ae99b139bfea19050ebbb44b38755b514f13b817"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f61247cb9452fae174bda563db35de002faaf422","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        \"\"\""},{"line_number":42,"context_line":"        # We import here to avoid conf loading order issues with cinder.service"},{"line_number":43,"context_line":"        # above."},{"line_number":44,"context_line":"        import cinder.backup.manager as backup_manager"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        CONF.register_opts(backup_manager.backup_manager_opts)"},{"line_number":47,"context_line":"        backup_driver \u003d CONF.backup_driver"}],"source_content_type":"text/x-python","patch_set":6,"id":"5fc1f717_d33d2cbd","line":44,"updated":"2019-03-29 13:46:25.000000000","message":"Hiding this here is breaking the docs build.  What was the problem with the way you had this on PS4, where the import was before line 23? (or did the problem only show up when you added the policy file check?)","commit_id":"74e3206a246067d3025bb773a3519f60e1ad568f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"198365da8b39bfdb2a13d8e489cc168d2b47b7c1","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        \"\"\""},{"line_number":42,"context_line":"        # We import here to avoid conf loading order issues with cinder.service"},{"line_number":43,"context_line":"        # above."},{"line_number":44,"context_line":"        import cinder.backup.manager as backup_manager"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        CONF.register_opts(backup_manager.backup_manager_opts)"},{"line_number":47,"context_line":"        backup_driver \u003d CONF.backup_driver"}],"source_content_type":"text/x-python","patch_set":6,"id":"5fc1f717_9601cd07","line":44,"in_reply_to":"5fc1f717_13e414e0","updated":"2019-03-29 18:20:30.000000000","message":"Looks like the culprit is [DEFAULT]/periodic_interval.  It\u0027s used by cinder.backup.manager and cinder.scheduler.manager and cinder.service, and defined in cinder.service.  We could pull out the cinder.service options to their own file and have each of backup/manager.py and scheduler/manager.py and service.py import it.  Or we could just do an out of order import and comment appropriately.","commit_id":"74e3206a246067d3025bb773a3519f60e1ad568f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8f621643fb2c8fe7653c51261f00aedab8b1a406","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        \"\"\""},{"line_number":42,"context_line":"        # We import here to avoid conf loading order issues with cinder.service"},{"line_number":43,"context_line":"        # above."},{"line_number":44,"context_line":"        import cinder.backup.manager as backup_manager"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        CONF.register_opts(backup_manager.backup_manager_opts)"},{"line_number":47,"context_line":"        backup_driver \u003d CONF.backup_driver"}],"source_content_type":"text/x-python","patch_set":6,"id":"5fc1f717_13e414e0","line":44,"in_reply_to":"5fc1f717_d33d2cbd","updated":"2019-03-29 14:01:12.000000000","message":"As mentioned in the comment above, importing this before service module cause problem in conf loading, you can get additional info by taking change on local and trying ``cinder-status upgrade check`` command","commit_id":"74e3206a246067d3025bb773a3519f60e1ad568f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5854e4658651a10fc98cf69bd24724e1eb4253dc","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        # above."},{"line_number":44,"context_line":"        import cinder.backup.manager as backup_manager"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        CONF.register_opts(backup_manager.backup_manager_opts)"},{"line_number":47,"context_line":"        backup_driver \u003d CONF.backup_driver"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        # Easy check in that a class name will have mixed casing"}],"source_content_type":"text/x-python","patch_set":6,"id":"5fc1f717_d01e90b9","line":46,"range":{"start_line":46,"start_character":8,"end_line":46,"end_character":62},"updated":"2019-03-28 20:18:19.000000000","message":"Since we already register opts when importing backup_manager[1], this doesn\u0027t seem necessary\n\n[1] https://github.com/openstack/cinder/blob/master/cinder/backup/manager.py#L84","commit_id":"74e3206a246067d3025bb773a3519f60e1ad568f"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7ee891d15525b883cd1aa4fced59285dcfbee8f4","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"# TODO(rosmaita): these are imported out of order because the backup.manager"},{"line_number":24,"context_line":"# uses config that is defined and loaded by cinder.service"},{"line_number":25,"context_line":"# flake8: noqa"},{"line_number":26,"context_line":"from cinder import service"},{"line_number":27,"context_line":"import cinder.backup.manager"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5fc1f717_5366ea2b","line":25,"updated":"2019-04-02 13:39:53.000000000","message":"I would rather do the inline import than disable flake8 checking for the entire file. That had been cleaned up before and was found to be hiding some legitimate issues elsewhere.","commit_id":"fd56ef9a4a48d76427d0ea8af97b478129e1226b"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"15cd7c41f8a33ca8e6ea6f6f59e420dd8a0e6a96","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"# TODO(rosmaita): these are imported out of order because the backup.manager"},{"line_number":24,"context_line":"# uses config that is defined and loaded by cinder.service"},{"line_number":25,"context_line":"# flake8: noqa"},{"line_number":26,"context_line":"from cinder import service"},{"line_number":27,"context_line":"import cinder.backup.manager"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5fc1f717_012b2330","line":25,"in_reply_to":"5fc1f717_5366ea2b","updated":"2019-04-02 17:47:00.000000000","message":"Fair enough.  I forgot that once the register_opts was removed from this file, having the backup_manager available was no longer an issue.  So the inline import is fine.","commit_id":"fd56ef9a4a48d76427d0ea8af97b478129e1226b"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d2dd553b1dd007efe12f94b3cfbab1a634d41f40","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        we now only allow the full driver path. This checks that there are not"},{"line_number":45,"context_line":"        any remaining settings using the old method."},{"line_number":46,"context_line":"        \"\"\""},{"line_number":47,"context_line":"        backup_driver \u003d CONF.backup_driver"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        # Easy check in that a class name will have mixed casing"},{"line_number":50,"context_line":"        if backup_driver \u003d\u003d backup_driver.lower():"}],"source_content_type":"text/x-python","patch_set":7,"id":"5fc1f717_13870212","line":47,"updated":"2019-04-02 13:25:01.000000000","message":"I don\u0027t want to overthink this, and what I\u0027m about to suggest should probably be part of the upgradecheck code itself (though it\u0027s too late to change that for Stein), but I wonder whether we should do a check here to make sure we\u0027re reading an actual config file rather than just the defaults (because of course the default value will always succeed).  We could do something like what you did on the policy_file patch to locate the dir to look for policy.json:\nhttps://review.openstack.org/#/c/647937/8/cinder/cmd/status.py@69","commit_id":"fd56ef9a4a48d76427d0ea8af97b478129e1226b"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7ee891d15525b883cd1aa4fced59285dcfbee8f4","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        we now only allow the full driver path. This checks that there are not"},{"line_number":45,"context_line":"        any remaining settings using the old method."},{"line_number":46,"context_line":"        \"\"\""},{"line_number":47,"context_line":"        backup_driver \u003d CONF.backup_driver"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        # Easy check in that a class name will have mixed casing"},{"line_number":50,"context_line":"        if backup_driver \u003d\u003d backup_driver.lower():"}],"source_content_type":"text/x-python","patch_set":7,"id":"5fc1f717_b35dce49","line":47,"in_reply_to":"5fc1f717_13870212","updated":"2019-04-02 13:39:53.000000000","message":"This entire tool assumes there are cinder settings to be checked. I don\u0027t think we need to go to great lengths in an individual checker to validate this. Just looking at the driver config setting should be sufficient.","commit_id":"fd56ef9a4a48d76427d0ea8af97b478129e1226b"}],"cinder/tests/unit/cmd/test_status.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"2e94280c8be7561614e7d9ada31d1ae090acad33","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        self._set_backup_driver(\u0027cinder.backup.drivers.swift\u0027)"},{"line_number":43,"context_line":"        result \u003d self.checks._check_backup_module()"},{"line_number":44,"context_line":"        self.assertEqual(uc.Code.FAILURE, result.code)"},{"line_number":45,"context_line":"        self.assertIn(\u0027requires the full path\u0027, result.details)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5fc1f717_73a98b8e","line":45,"range":{"start_line":45,"start_character":22,"end_line":45,"end_character":46},"updated":"2019-04-03 14:46:47.000000000","message":"regarding my comment here https://review.openstack.org/#/c/648149/1/cinder/tests/unit/cmd/test_status.py@61","commit_id":"cef38b5e849bad294374adaf213efabfc26b0d5c"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"bf3d6510835427f97beded2ed58d54494901c450","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        self._set_backup_driver(\u0027cinder.backup.drivers.swift\u0027)"},{"line_number":43,"context_line":"        result \u003d self.checks._check_backup_module()"},{"line_number":44,"context_line":"        self.assertEqual(uc.Code.FAILURE, result.code)"},{"line_number":45,"context_line":"        self.assertIn(\u0027requires the full path\u0027, result.details)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5fc1f717_93e8b7b3","line":45,"range":{"start_line":45,"start_character":22,"end_line":45,"end_character":46},"in_reply_to":"5fc1f717_73a98b8e","updated":"2019-04-03 14:51:11.000000000","message":"I did take it out but decided to add it back in since we do want a basic check for some key detail. I definitely would not want to validate the entire string.","commit_id":"cef38b5e849bad294374adaf213efabfc26b0d5c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"badb9d2d829216807166497bd44bda4164009747","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        self._set_backup_driver(\u0027cinder.backup.drivers.swift\u0027)"},{"line_number":43,"context_line":"        result \u003d self.checks._check_backup_module()"},{"line_number":44,"context_line":"        self.assertEqual(uc.Code.FAILURE, result.code)"},{"line_number":45,"context_line":"        self.assertIn(\u0027requires the full path\u0027, result.details)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5fc1f717_53ab2f1d","line":45,"range":{"start_line":45,"start_character":22,"end_line":45,"end_character":46},"in_reply_to":"5fc1f717_93e8b7b3","updated":"2019-04-03 15:02:46.000000000","message":"An assert is required indeed, Regarding my last comment there,\nkeeping a constant string in result details to assert.","commit_id":"cef38b5e849bad294374adaf213efabfc26b0d5c"}]}