)]}'
{"cinder/api/contrib/admin_actions.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"51b991b26657ba31d78905faf9dfdcdf10299735","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        valid \u003d False"},{"line_number":161,"context_line":"        if status:"},{"line_number":162,"context_line":"            valid \u003d True"},{"line_number":163,"context_line":"            if not isinstance(status, (str, unicode)):"},{"line_number":164,"context_line":"                raise exc.HTTPBadRequest("},{"line_number":165,"context_line":"                    explanation\u003d_(\"Value for status must be a string\"))"},{"line_number":166,"context_line":"            update \u003d super(VolumeAdminController, self).validate_update(body)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_0d9e5d56","line":163,"updated":"2016-09-13 19:06:33.000000000","message":"You need\n\nif isinstance(value, six.string_types):","commit_id":"0e2d9343b3e406dd7b2623d363470f2b1c65ddf8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"28cd5476185025e314fd7d154261cbf96285f38c","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        return self.volume_api.delete(*args, **kwargs)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def validate_update(self, body):"},{"line_number":156,"context_line":"        update \u003d {}"},{"line_number":157,"context_line":"        status \u003d body.get(\u0027status\u0027, None)"},{"line_number":158,"context_line":"        attach_status \u003d body.get(\u0027attach_status\u0027, None)"},{"line_number":159,"context_line":"        migration_status \u003d body.get(\u0027migration_status\u0027, None)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_9df5a858","line":156,"updated":"2016-09-26 10:03:51.000000000","message":"We can refactor this whole method like this:\n\n check \u003d {\u0027status\u0027: self.valid_status,\n          \u0027attach_status\u0027: self.valid_attach_status,\n          \u0027migration_status\u0027: self.valid_migration_status}\n update \u003d {}\n\n for attr, valid in check.items():\n     value \u003d body.get(attr)\n     value \u003d value and six.text_type(value).lower()\n     if value:\n         if value not in valid():\n             raise InvalidInput(reason\u003d_(\u0027Must specify a valid %s status\u0027) % value)\n         update[attr] \u003d value\n\n if update.get(\u0027migration_status\u0027) \u003d\u003d \u0027none\u0027:\n     update[\u0027migration_status\u0027 \u003d None\n\n if not update:\n     raise MissingRequired(element\u003dutils.build_or_str(check.keys())\n\n return udpate","commit_id":"1ca27195e77a244a1c0c741481ec68450635af30"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"fa9c0429f9b7e9391dfed4637875523eac8ef521","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        valid \u003d False"},{"line_number":162,"context_line":"        if status:"},{"line_number":163,"context_line":"            valid \u003d True"},{"line_number":164,"context_line":"            if not isinstance(status, six.string_types):"},{"line_number":165,"context_line":"                raise exc.HTTPBadRequest("},{"line_number":166,"context_line":"                    explanation\u003d_(\"Value for status must be a string\"))"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8ec9b2_030df03d","line":163,"range":{"start_line":163,"start_character":12,"end_line":163,"end_character":17},"updated":"2016-09-20 22:19:01.000000000","message":"seems like the valid variable is redundant, we shouldn\u0027t ever get to L#192 because these would raise and exit out, no?\n\nAlso should we *care* what status fields these are (ie do we need the different cases), this just seems harder to maintain that it\u0027s worth.  Maybe refactor the validate_update to just let the caller send in the status value to check... or even better get rid of the over-ridden impl here in the Volume controller, make a generic \u0027validate_string\u0027 in the base class and use it, let the caller figure out what to catch/raise etc.","commit_id":"1ca27195e77a244a1c0c741481ec68450635af30"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"1ff5f8834de18f73e9a391ce1d6987554aaec005","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        valid \u003d False"},{"line_number":162,"context_line":"        if status:"},{"line_number":163,"context_line":"            valid \u003d True"},{"line_number":164,"context_line":"            if not isinstance(status, six.string_types):"},{"line_number":165,"context_line":"                raise exc.HTTPBadRequest("},{"line_number":166,"context_line":"                    explanation\u003d_(\"Value for status must be a string\"))"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8ec9b2_2e78132b","line":163,"range":{"start_line":163,"start_character":12,"end_line":163,"end_character":17},"in_reply_to":"7a8ec9b2_030df03d","updated":"2016-09-20 22:30:22.000000000","message":"Oh, never mind... you\u0027ve added the dependency on the status enums.","commit_id":"1ca27195e77a244a1c0c741481ec68450635af30"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"28cd5476185025e314fd7d154261cbf96285f38c","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        if status:"},{"line_number":163,"context_line":"            valid \u003d True"},{"line_number":164,"context_line":"            if not isinstance(status, six.string_types):"},{"line_number":165,"context_line":"                raise exc.HTTPBadRequest("},{"line_number":166,"context_line":"                    explanation\u003d_(\"Value for status must be a string\"))"},{"line_number":167,"context_line":"            update \u003d super(VolumeAdminController, self).validate_update(body)"},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_5db5e02e","line":165,"updated":"2016-09-26 10:03:51.000000000","message":"nit: Please raise InvalidInput instead, it will be converted by our middleware to the right HTTP response code.","commit_id":"1ca27195e77a244a1c0c741481ec68450635af30"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"28cd5476185025e314fd7d154261cbf96285f38c","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        if attach_status:"},{"line_number":170,"context_line":"            valid \u003d True"},{"line_number":171,"context_line":"            if not isinstance(attach_status, six.string_types):"},{"line_number":172,"context_line":"                raise exc.HTTPBadRequest("},{"line_number":173,"context_line":"                    explanation\u003d_(\"Value for attach_status must be a string\"))"},{"line_number":174,"context_line":"            update[\u0027attach_status\u0027] \u003d attach_status.lower()"},{"line_number":175,"context_line":"            if update[\u0027attach_status\u0027] not in self.valid_attach_status:"},{"line_number":176,"context_line":"                raise exc.HTTPBadRequest("}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_1d20b847","line":173,"range":{"start_line":171,"start_character":0,"end_line":173,"end_character":78},"updated":"2016-09-26 10:03:51.000000000","message":"nit: This is unnecessary, you can just do:\n\n update[\u0027attach_status\u0027] \u003d six.text_type(attach_status).lower()\n\nThat way we don\u0027t have 2 checks","commit_id":"1ca27195e77a244a1c0c741481ec68450635af30"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"28cd5476185025e314fd7d154261cbf96285f38c","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        if migration_status:"},{"line_number":180,"context_line":"            valid \u003d True"},{"line_number":181,"context_line":"            if not isinstance(migration_status, six.string_types):"},{"line_number":182,"context_line":"                raise exc.HTTPBadRequest("},{"line_number":183,"context_line":"                    explanation\u003d"},{"line_number":184,"context_line":"                    _(\"Value for migration_status must be a string\"))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_9d96883d","line":181,"updated":"2016-09-26 10:03:51.000000000","message":"ditto","commit_id":"1ca27195e77a244a1c0c741481ec68450635af30"}]}