)]}'
{"glance/cmd/manage.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"57c0c5ab6bc1f180aca4bcf89bdcbe13c304b6e7","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        sys.exit(\"ERROR: %s\" % e)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    func \u003d CONF.command.action_fn"},{"line_number":189,"context_line":"    func_args \u003d [arg.decode(\u0027utf-8\u0027) for arg in CONF.command.action_args]"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F91%2BA%3D","line":189,"updated":"2013-12-05 10:45:22.000000000","message":"leverage glance.openstack.common.strutils.safe_decode()? since it can involve stdin\u0027s encoding: https://github.com/openstack/glance/blob/master/glance/openstack/common/strutils.py#L99","commit_id":"f4f02019532dcc161e38a9c53fc2d7e2f03093f8"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"57c0c5ab6bc1f180aca4bcf89bdcbe13c304b6e7","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    try:"},{"line_number":193,"context_line":"        ret \u003d func(*func_args)"},{"line_number":194,"context_line":"        return(ret)"},{"line_number":195,"context_line":"    except exception.GlanceException as e:"},{"line_number":196,"context_line":"        sys.exit(\"ERROR: %s\" % e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F91%2Fo%3D","line":193,"updated":"2013-12-05 10:45:22.000000000","message":"nit: return func(*func_args) ?","commit_id":"f4f02019532dcc161e38a9c53fc2d7e2f03093f8"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"e05f02ba7f005cb700af1230b4c950ab821b9e19","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                v \u003d getattr(CONF.command, \u0027action_kwarg_\u0027 + k)"},{"line_number":225,"context_line":"                if v is None:"},{"line_number":226,"context_line":"                    continue"},{"line_number":227,"context_line":"                strutils.safe_decode(v)"},{"line_number":228,"context_line":"                func_kwargs[k] \u003d v"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"            func_args \u003d [strutils.safe_decode(arg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9oqI%3D","line":227,"updated":"2013-12-09 13:48:44.000000000","message":"v \u003d ...","commit_id":"035a60e1dd826d8722132e7acd81cbe6e9aefad2"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"81f9242d0c42ccfa6f18e9e33baa8744bc35f411","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                v \u003d getattr(CONF.command, \u0027action_kwarg_\u0027 + k)"},{"line_number":225,"context_line":"                if v is None:"},{"line_number":226,"context_line":"                    continue"},{"line_number":227,"context_line":"                strutils.safe_decode(v)"},{"line_number":228,"context_line":"                func_kwargs[k] \u003d v"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"            func_args \u003d [strutils.safe_decode(arg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9ojg%3D","line":227,"in_reply_to":"AAAATn%2F9onY%3D","updated":"2013-12-09 14:05:47.000000000","message":"I mean we need to save the decode result back to var \"v\":\n\nv \u003d strutils.safe_decode(v)\n\nactually I think we can merge this line with below line.","commit_id":"035a60e1dd826d8722132e7acd81cbe6e9aefad2"},{"author":{"_account_id":7701,"name":"Amala Basha","email":"amala.alungal@RACKSPACE.COM","username":"amalabasha"},"change_message_id":"eca008b776f4ee193775ef0f67397578e15a7a50","unresolved":false,"context_lines":[{"line_number":224,"context_line":"                v \u003d getattr(CONF.command, \u0027action_kwarg_\u0027 + k)"},{"line_number":225,"context_line":"                if v is None:"},{"line_number":226,"context_line":"                    continue"},{"line_number":227,"context_line":"                strutils.safe_decode(v)"},{"line_number":228,"context_line":"                func_kwargs[k] \u003d v"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"            func_args \u003d [strutils.safe_decode(arg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9onY%3D","line":227,"in_reply_to":"AAAATn%2F9oqI%3D","updated":"2013-12-09 13:52:28.000000000","message":"Ummm I\u0027m not sure I got that.","commit_id":"035a60e1dd826d8722132e7acd81cbe6e9aefad2"},{"author":{"_account_id":177,"name":"Alex Meade","email":"mr.alex.meade@gmail.com","username":"alex-meade"},"change_message_id":"6c1a4ccf55898f7f1625fbf2dff2cd473d2358fc","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        sys.exit(\"ERROR: %s\" % e)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    try:"},{"line_number":219,"context_line":"        if CONF.command.action.startswith(\u0027db\u0027):"},{"line_number":220,"context_line":"            return CONF.command.action_fn()"},{"line_number":221,"context_line":"        else:"},{"line_number":222,"context_line":"            func_kwargs \u003d {}"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9anw%3D","line":219,"updated":"2013-12-11 19:32:58.000000000","message":"Is this a check to see if it\u0027s a legacy command? could that be more explicit?","commit_id":"a81c4bf53995c962e0f26c9977850375076fa8bb"},{"author":{"_account_id":177,"name":"Alex Meade","email":"mr.alex.meade@gmail.com","username":"alex-meade"},"change_message_id":"6c1a4ccf55898f7f1625fbf2dff2cd473d2358fc","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        if CONF.command.action.startswith(\u0027db\u0027):"},{"line_number":220,"context_line":"            return CONF.command.action_fn()"},{"line_number":221,"context_line":"        else:"},{"line_number":222,"context_line":"            func_kwargs \u003d {}"},{"line_number":223,"context_line":"            for k in CONF.command.action_kwargs:"},{"line_number":224,"context_line":"                v \u003d getattr(CONF.command, \u0027action_kwarg_\u0027 + k)"},{"line_number":225,"context_line":"                if v is None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9anU%3D","line":222,"updated":"2013-12-11 19:32:58.000000000","message":"Could this code block be in it\u0027s own function like \u0027decode_args\u0027 or something so that it is more obvious at a glance to see what this is doing?","commit_id":"a81c4bf53995c962e0f26c9977850375076fa8bb"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"c268811ba7a882337aeb8f8ea8b05f02eef0fba1","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        migration.db_sync(version, current_version)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"class DbLegacyCommands(object):"},{"line_number":98,"context_line":"    \"\"\"Class for managing the db using legacy commands\"\"\""},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def __init__(self, command_object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9Rfg%3D","line":97,"updated":"2013-12-13 07:56:20.000000000","message":"Could you please explain - perhaps add it to the commit message - why this class is needed?","commit_id":"3749495fcd3ef7711c6167970102740fd3fb0dfc"},{"author":{"_account_id":7701,"name":"Amala Basha","email":"amala.alungal@RACKSPACE.COM","username":"amalabasha"},"change_message_id":"df1a84ea645d6c639ffa362fa3d6f2d895a9b3fa","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        migration.db_sync(version, current_version)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"class DbLegacyCommands(object):"},{"line_number":98,"context_line":"    \"\"\"Class for managing the db using legacy commands\"\"\""},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def __init__(self, command_object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9I0I%3D","line":97,"in_reply_to":"AAAATn%2F9Rfg%3D","updated":"2013-12-16 12:59:31.000000000","message":"Hi Flavio, This is just to ensure that backward compatibility is not broken. The way the old code dealt with it is slightly different. Hence this approach","commit_id":"3749495fcd3ef7711c6167970102740fd3fb0dfc"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"c268811ba7a882337aeb8f8ea8b05f02eef0fba1","unresolved":false,"context_lines":[{"line_number":215,"context_line":"    except RuntimeError as e:"},{"line_number":216,"context_line":"        sys.exit(\"ERROR: %s\" % e)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    try:"},{"line_number":219,"context_line":"        if CONF.command.action.startswith(\u0027db\u0027):"},{"line_number":220,"context_line":"            return CONF.command.action_fn()"},{"line_number":221,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9RfE%3D","line":218,"updated":"2013-12-13 07:56:20.000000000","message":"Why is all the code below needed? Isn\u0027t it duplicating what we already have in `add_command_parsers` ?","commit_id":"3749495fcd3ef7711c6167970102740fd3fb0dfc"},{"author":{"_account_id":7701,"name":"Amala Basha","email":"amala.alungal@RACKSPACE.COM","username":"amalabasha"},"change_message_id":"df1a84ea645d6c639ffa362fa3d6f2d895a9b3fa","unresolved":false,"context_lines":[{"line_number":215,"context_line":"    except RuntimeError as e:"},{"line_number":216,"context_line":"        sys.exit(\"ERROR: %s\" % e)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    try:"},{"line_number":219,"context_line":"        if CONF.command.action.startswith(\u0027db\u0027):"},{"line_number":220,"context_line":"            return CONF.command.action_fn()"},{"line_number":221,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9Iz0%3D","line":218,"in_reply_to":"AAAATn%2F9RfE%3D","updated":"2013-12-16 12:59:31.000000000","message":"Its not really duplicating it. This is the bit which deals with picking up the optional params supplied at the cli and matching it with the registered optional arg added by the decorator in the code above.","commit_id":"3749495fcd3ef7711c6167970102740fd3fb0dfc"}],"glance/tests/unit/test_manage.py":[{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"907dd71e71a532b646606a822fe7f4fa0ded3ae5","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def setUp(self):"},{"line_number":31,"context_line":"        super(TestLegacyManage, self).setUp()"},{"line_number":32,"context_line":"        self.temp \u003d cfg.CONF"},{"line_number":33,"context_line":"        NEWCONF \u003d cfg.ConfigOpts()"},{"line_number":34,"context_line":"        manage.CONF \u003d NEWCONF"},{"line_number":35,"context_line":"        glance.common.config.CONF \u003d NEWCONF"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9PAA%3D","line":32,"updated":"2013-12-13 17:31:24.000000000","message":"I believe the preferred way to handle overriding conf settings is to use the config fixture: https://github.com/openstack/glance/blob/master/glance/openstack/common/fixture/config.py","commit_id":"3749495fcd3ef7711c6167970102740fd3fb0dfc"},{"author":{"_account_id":7701,"name":"Amala Basha","email":"amala.alungal@RACKSPACE.COM","username":"amalabasha"},"change_message_id":"df1a84ea645d6c639ffa362fa3d6f2d895a9b3fa","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def setUp(self):"},{"line_number":31,"context_line":"        super(TestLegacyManage, self).setUp()"},{"line_number":32,"context_line":"        self.temp \u003d cfg.CONF"},{"line_number":33,"context_line":"        NEWCONF \u003d cfg.ConfigOpts()"},{"line_number":34,"context_line":"        manage.CONF \u003d NEWCONF"},{"line_number":35,"context_line":"        glance.common.config.CONF \u003d NEWCONF"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9Iyo%3D","line":32,"in_reply_to":"AAAATn%2F9PAA%3D","updated":"2013-12-16 12:59:31.000000000","message":"Hi Ben, Thanks a lot for the review. but even with the fixture in place, the oslo.cfg.reset() does not clear all the registered options. We\u0027ve a bug filed at https://bugs.launchpad.net/oslo/+bug/1261376. SO for now, since this is a needed fix, Im thinking of pushing this without the tests. Tests could probably be added later once the oslo bug is fixed and merged. Thoughts?","commit_id":"3749495fcd3ef7711c6167970102740fd3fb0dfc"}]}