)]}'
{"glance/api/policy.py":[{"author":{"_account_id":7680,"name":"Alex Gaynor","email":"alex.gaynor@gmail.com","username":"alex_gaynor"},"change_message_id":"6b96c3a7924467d55e9343dbe1796975d8c2f851","unresolved":false,"context_lines":[{"line_number":291,"context_line":"        self.context \u003d context"},{"line_number":292,"context_line":"        self.policy \u003d policy"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"    @staticmethod"},{"line_number":295,"context_line":"    def _get_checker(action, func_name):"},{"line_number":296,"context_line":"        def _checker(self, *args, **kwargs):"},{"line_number":297,"context_line":"            self.policy.enforce(self.context, action, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPn%2F%2FdfI%3D","line":294,"updated":"2013-07-01 14:04:30.000000000","message":"Rather than marking this as a staticmethod, and then needing to use __func__ below, this would probably be better without @staticmethod, and then `del _get_checker` to remove it from the namespace.","commit_id":"371c265fe74603a6580a3e9d2291ce5d07421615"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"d56134afa4052308b061d52ac3cd5d6fed7c30a7","unresolved":false,"context_lines":[{"line_number":291,"context_line":"        self.context \u003d context"},{"line_number":292,"context_line":"        self.policy \u003d policy"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"    @staticmethod"},{"line_number":295,"context_line":"    def _get_checker(action, func_name):"},{"line_number":296,"context_line":"        def _checker(self, *args, **kwargs):"},{"line_number":297,"context_line":"            self.policy.enforce(self.context, action, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPn%2F%2Fbi8%3D","line":294,"in_reply_to":"AAAAPn%2F%2FdfI%3D","updated":"2013-07-02 05:15:46.000000000","message":"Done, thanks Alex.","commit_id":"371c265fe74603a6580a3e9d2291ce5d07421615"}],"glance/api/v2/images.py":[{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        path_root \u003d path[0]"},{"line_number":129,"context_line":"        value \u003d change[\u0027value\u0027]"},{"line_number":130,"context_line":"        if path_root \u003d\u003d \u0027locations\u0027:"},{"line_number":131,"context_line":"            if len(image.locations) \u003e 0 and len(value) \u003e 0:"},{"line_number":132,"context_line":"                msg \u003d _(\"Cannot replace locations from a non-empty \""},{"line_number":133,"context_line":"                        \"list to a non-empty list.\")"},{"line_number":134,"context_line":"                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FL4%3D","line":131,"updated":"2013-07-07 18:43:41.000000000","message":"It might help reduce complexity and function length to put the logic starting here into its own \"_do_replace_locations\" function.\n\nOr maybe even \n\n  if path_root \u003d\u003d \u0027locations\u0027:\n      if len(path) \u003d\u003d 1:\n          _do_replace_locations(...)\n      else:\n          _do_replace_location(...)\n\nBut that\u0027s just an idle suggestion, do whatever you think looks best to reduce the complexity and function length here.","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        path_root \u003d path[0]"},{"line_number":129,"context_line":"        value \u003d change[\u0027value\u0027]"},{"line_number":130,"context_line":"        if path_root \u003d\u003d \u0027locations\u0027:"},{"line_number":131,"context_line":"            if len(image.locations) \u003e 0 and len(value) \u003e 0:"},{"line_number":132,"context_line":"                msg \u003d _(\"Cannot replace locations from a non-empty \""},{"line_number":133,"context_line":"                        \"list to a non-empty list.\")"},{"line_number":134,"context_line":"                raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2Bnw%3D","line":131,"in_reply_to":"AAAAP3%2F%2F%2FL4%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                except ValueError:"},{"line_number":148,"context_line":"                    pass    # update image status failed."},{"line_number":149,"context_line":"        else:"},{"line_number":150,"context_line":"            if hasattr(image, path_root):"},{"line_number":151,"context_line":"                setattr(image, path_root, value)"},{"line_number":152,"context_line":"            elif path_root in image.extra_properties:"},{"line_number":153,"context_line":"                image.extra_properties[path_root] \u003d value"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLw%3D","line":150,"updated":"2013-07-07 18:43:41.000000000","message":"I understand that the deserializer is doing appropriate filtering to make sure paths don\u0027t actually have len \u003e 1 here, but it is very confusing to call this path_root, to me. It seems to imply that there are other parts to the path, and then the code goes on to ignore those other parts. This isn\u0027t really a blocker, but if you can figure out a way to improve this I\u0027d be really happy :-)","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        if path_root \u003d\u003d \u0027locations\u0027:"},{"line_number":163,"context_line":"            # NOTE(zhiyan): Don\u0027t allow client add a list in one call"},{"line_number":164,"context_line":"            value \u003d unicode(value)"},{"line_number":165,"context_line":"            if value in image.locations:"},{"line_number":166,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is already a registered location.\" % value)"},{"line_number":167,"context_line":"                raise webob.exc.HTTPConflict(msg)"},{"line_number":168,"context_line":"            pos \u003d len(image.locations)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLs%3D","line":165,"updated":"2013-07-07 18:43:41.000000000","message":"In the end, I don\u0027t think there is much point in doing this check.\n\nIts entirely possible for two store urls to be different lexicographically, but be identical semantically. For example\n\n  swift://foo/bar\n  swift+https://foo/bar\n\nThese both point to the same thing.","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        if path_root \u003d\u003d \u0027locations\u0027:"},{"line_number":163,"context_line":"            # NOTE(zhiyan): Don\u0027t allow client add a list in one call"},{"line_number":164,"context_line":"            value \u003d unicode(value)"},{"line_number":165,"context_line":"            if value in image.locations:"},{"line_number":166,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is already a registered location.\" % value)"},{"line_number":167,"context_line":"                raise webob.exc.HTTPConflict(msg)"},{"line_number":168,"context_line":"            pos \u003d len(image.locations)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2BvU%3D","line":165,"in_reply_to":"AAAAP3%2F%2F%2FLs%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            if value in image.locations:"},{"line_number":166,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is already a registered location.\" % value)"},{"line_number":167,"context_line":"                raise webob.exc.HTTPConflict(msg)"},{"line_number":168,"context_line":"            pos \u003d len(image.locations)"},{"line_number":169,"context_line":"            if (len(path) \u003e 1 and path[1].isdigit()):"},{"line_number":170,"context_line":"                # NOTE(zhiyan): locations index from \u00271\u0027 by client perspective"},{"line_number":171,"context_line":"                pos \u003d int(path[1]) - 1"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLI%3D","line":168,"updated":"2013-07-07 18:43:41.000000000","message":"pos should only be len(image.locations) if len(path) \u003d\u003d 2 and path[1] \u003d\u003d \u0027-\u0027. I cannot find where those restrictions are being enforced. I was assuming such restrictions would be either here, or down in the validation checks, can you point them out to me?","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            if value in image.locations:"},{"line_number":166,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is already a registered location.\" % value)"},{"line_number":167,"context_line":"                raise webob.exc.HTTPConflict(msg)"},{"line_number":168,"context_line":"            pos \u003d len(image.locations)"},{"line_number":169,"context_line":"            if (len(path) \u003e 1 and path[1].isdigit()):"},{"line_number":170,"context_line":"                # NOTE(zhiyan): locations index from \u00271\u0027 by client perspective"},{"line_number":171,"context_line":"                pos \u003d int(path[1]) - 1"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2Buk%3D","line":168,"in_reply_to":"AAAAP3%2F%2F%2FLI%3D","updated":"2013-07-08 08:34:47.000000000","message":"Adding the necessary checking to RequestDeserializer._validate_path() and refactoring this code in next PS.","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is already a registered location.\" % value)"},{"line_number":167,"context_line":"                raise webob.exc.HTTPConflict(msg)"},{"line_number":168,"context_line":"            pos \u003d len(image.locations)"},{"line_number":169,"context_line":"            if (len(path) \u003e 1 and path[1].isdigit()):"},{"line_number":170,"context_line":"                # NOTE(zhiyan): locations index from \u00271\u0027 by client perspective"},{"line_number":171,"context_line":"                pos \u003d int(path[1]) - 1"},{"line_number":172,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLE%3D","line":169,"updated":"2013-07-07 18:43:41.000000000","message":"_do_add for locations should *only* work if path \u003d\u003d 2. Anything else is invalid. Is that being enforced somewhere? I searched but couldn\u0027t quite find it.","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is already a registered location.\" % value)"},{"line_number":167,"context_line":"                raise webob.exc.HTTPConflict(msg)"},{"line_number":168,"context_line":"            pos \u003d len(image.locations)"},{"line_number":169,"context_line":"            if (len(path) \u003e 1 and path[1].isdigit()):"},{"line_number":170,"context_line":"                # NOTE(zhiyan): locations index from \u00271\u0027 by client perspective"},{"line_number":171,"context_line":"                pos \u003d int(path[1]) - 1"},{"line_number":172,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2BuY%3D","line":169,"in_reply_to":"AAAAP3%2F%2F%2FLE%3D","updated":"2013-07-08 08:34:47.000000000","message":"Yes, that is. I will adding strict checking for path depth limitations to RequestDeserializer._validate_path() in next PS.","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        path_root \u003d path[0]"},{"line_number":189,"context_line":"        if path_root \u003d\u003d \u0027locations\u0027:"},{"line_number":190,"context_line":"            pos \u003d None"},{"line_number":191,"context_line":"            if (len(path) \u003e 1 and path[1].isdigit()):"},{"line_number":192,"context_line":"                # NOTE(zhiyan): locations index from \u00271\u0027 by client perspective"},{"line_number":193,"context_line":"                pos \u003d int(path[1]) - 1"},{"line_number":194,"context_line":"            if (pos is None) or (pos not in range(len(image.locations))):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLA%3D","line":191,"updated":"2013-07-07 18:43:41.000000000","message":"It looks like this logic *might* be duplicated in other functions above. I only mention that because it could help reduce the complexity and function length if the code can be sensibly deduplicated. Its your call, of course.","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        path_root \u003d path[0]"},{"line_number":189,"context_line":"        if path_root \u003d\u003d \u0027locations\u0027:"},{"line_number":190,"context_line":"            pos \u003d None"},{"line_number":191,"context_line":"            if (len(path) \u003e 1 and path[1].isdigit()):"},{"line_number":192,"context_line":"                # NOTE(zhiyan): locations index from \u00271\u0027 by client perspective"},{"line_number":193,"context_line":"                pos \u003d int(path[1]) - 1"},{"line_number":194,"context_line":"            if (pos is None) or (pos not in range(len(image.locations))):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2Bpk%3D","line":191,"in_reply_to":"AAAAP3%2F%2F%2FLA%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                        \u0027disk_format\u0027, \u0027id\u0027, \u0027min_disk\u0027, \u0027min_ram\u0027, \u0027name\u0027,"},{"line_number":239,"context_line":"                        \u0027size\u0027, \u0027status\u0027, \u0027tags\u0027, \u0027updated_at\u0027, \u0027visibility\u0027,"},{"line_number":240,"context_line":"                        \u0027protected\u0027]"},{"line_number":241,"context_line":"    _path_deep_limits \u003d {\u0027locations\u0027: 2}"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def __init__(self, schema\u003dNone):"},{"line_number":244,"context_line":"        super(RequestDeserializer, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLk%3D","line":241,"updated":"2013-07-07 18:43:41.000000000","message":"For slightly better wording, I think this should be \"_path_depth_limits\".","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                        \u0027disk_format\u0027, \u0027id\u0027, \u0027min_disk\u0027, \u0027min_ram\u0027, \u0027name\u0027,"},{"line_number":239,"context_line":"                        \u0027size\u0027, \u0027status\u0027, \u0027tags\u0027, \u0027updated_at\u0027, \u0027visibility\u0027,"},{"line_number":240,"context_line":"                        \u0027protected\u0027]"},{"line_number":241,"context_line":"    _path_deep_limits \u003d {\u0027locations\u0027: 2}"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def __init__(self, schema\u003dNone):"},{"line_number":244,"context_line":"        super(RequestDeserializer, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2Bv0%3D","line":241,"in_reply_to":"AAAAP3%2F%2F%2FLk%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":319,"context_line":"        \"\"\""},{"line_number":320,"context_line":"        self._validate_json_pointer(pointer)"},{"line_number":321,"context_line":"        ret \u003d []"},{"line_number":322,"context_line":"        for p in pointer.lstrip(\u0027/\u0027).split(\u0027/\u0027):"},{"line_number":323,"context_line":"            ret.append(p.replace(\u0027~1\u0027, \u0027/\u0027).replace(\u0027~0\u0027, \u0027~\u0027).strip())"},{"line_number":324,"context_line":"        return ret"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLg%3D","line":322,"updated":"2013-07-07 18:43:41.000000000","message":"Some time now or in the future, pylint will complain about this single letter variable. Maybe something longer, like \"part\" ?","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":319,"context_line":"        \"\"\""},{"line_number":320,"context_line":"        self._validate_json_pointer(pointer)"},{"line_number":321,"context_line":"        ret \u003d []"},{"line_number":322,"context_line":"        for p in pointer.lstrip(\u0027/\u0027).split(\u0027/\u0027):"},{"line_number":323,"context_line":"            ret.append(p.replace(\u0027~1\u0027, \u0027/\u0027).replace(\u0027~0\u0027, \u0027~\u0027).strip())"},{"line_number":324,"context_line":"        return ret"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2BwU%3D","line":322,"in_reply_to":"AAAAP3%2F%2F%2FLg%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        \"\"\" Validate a json pointer."},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        We only accept a limited form of json pointers. Specifically, we do"},{"line_number":330,"context_line":"        not allow multiple levels of indirection, so there can only be one \u0027/\u0027"},{"line_number":331,"context_line":"        in the pointer, located at the start of the string."},{"line_number":332,"context_line":"        \"\"\""},{"line_number":333,"context_line":"        if not pointer.startswith(\u0027/\u0027):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLY%3D","line":330,"updated":"2013-07-07 18:43:41.000000000","message":"This comment is wrong now.","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        \"\"\" Validate a json pointer."},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        We only accept a limited form of json pointers. Specifically, we do"},{"line_number":330,"context_line":"        not allow multiple levels of indirection, so there can only be one \u0027/\u0027"},{"line_number":331,"context_line":"        in the pointer, located at the start of the string."},{"line_number":332,"context_line":"        \"\"\""},{"line_number":333,"context_line":"        if not pointer.startswith(\u0027/\u0027):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2BwQ%3D","line":330,"in_reply_to":"AAAAP3%2F%2F%2FLY%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":334,"context_line":"            msg \u003d _(\u0027Pointer `%s` does not start with \"/\".\u0027 % pointer)"},{"line_number":335,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":336,"context_line":"        if re.search(\u0027/\\s*?/\u0027, pointer[1:]):"},{"line_number":337,"context_line":"            msg \u003d _(\u0027Pointer `%s` contains continuous \"/\".\u0027 % pointer)"},{"line_number":338,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":339,"context_line":"        if len(pointer) \u003e 1 and pointer.endswith(\u0027/\u0027):"},{"line_number":340,"context_line":"            msg \u003d _(\u0027Pointer `%s` end with \"/\".\u0027 % pointer)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLM%3D","line":337,"updated":"2013-07-07 18:43:41.000000000","message":"maybe s/continous/adjacent/ or s/continuous/repeated/ ?","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":334,"context_line":"            msg \u003d _(\u0027Pointer `%s` does not start with \"/\".\u0027 % pointer)"},{"line_number":335,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":336,"context_line":"        if re.search(\u0027/\\s*?/\u0027, pointer[1:]):"},{"line_number":337,"context_line":"            msg \u003d _(\u0027Pointer `%s` contains continuous \"/\".\u0027 % pointer)"},{"line_number":338,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":339,"context_line":"        if len(pointer) \u003e 1 and pointer.endswith(\u0027/\u0027):"},{"line_number":340,"context_line":"            msg \u003d _(\u0027Pointer `%s` end with \"/\".\u0027 % pointer)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2BwI%3D","line":337,"in_reply_to":"AAAAP3%2F%2F%2FLM%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":342,"context_line":"        if pointer[1:].strip() \u003d\u003d \u0027/\u0027:"},{"line_number":343,"context_line":"            msg \u003d _(\u0027Pointer `%s` does not contains valid token.\u0027 % pointer)"},{"line_number":344,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":345,"context_line":"        if re.search(\u0027~[^01]\u0027, pointer) or pointer.endswith(\u0027~\u0027):"},{"line_number":346,"context_line":"            msg \u003d _(\u0027Pointer `%s` contains \"~\" not part of\u0027"},{"line_number":347,"context_line":"                    \u0027 a recognized escape sequence.\u0027 % pointer)"},{"line_number":348,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLQ%3D","line":345,"updated":"2013-07-07 18:43:41.000000000","message":"Ooh good catch!","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"bb3ed21ce4d8e0ac0e22ff251d94ea753e0a1702","unresolved":false,"context_lines":[{"line_number":375,"context_line":"    def _validate_path(self, path):"},{"line_number":376,"context_line":"        path_root \u003d path[0]"},{"line_number":377,"context_line":"        if len(path) \u003e self._path_deep_limits.get(path_root, 1):"},{"line_number":378,"context_line":"            msg \u003d \"Path of attribute \\\u0027%s\\\u0027 is invalid.\" % path_root"},{"line_number":379,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(msg))"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"    def _parse_json_schema_change(self, raw_change, draft_version):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2FLc%3D","line":378,"updated":"2013-07-07 18:43:41.000000000","message":"I think you should use the whole path in the message here.\n\n  msg \u003d \"Invalid JSON pointer for this resource: \\\u0027/%s\\\u0027\" % \u0027/\u0027.join(path)","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"fcc76e9806e8226075ab55ee8171ceaa791ee124","unresolved":false,"context_lines":[{"line_number":375,"context_line":"    def _validate_path(self, path):"},{"line_number":376,"context_line":"        path_root \u003d path[0]"},{"line_number":377,"context_line":"        if len(path) \u003e self._path_deep_limits.get(path_root, 1):"},{"line_number":378,"context_line":"            msg \u003d \"Path of attribute \\\u0027%s\\\u0027 is invalid.\" % path_root"},{"line_number":379,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(msg))"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"    def _parse_json_schema_change(self, raw_change, draft_version):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2F%2Bv8%3D","line":378,"in_reply_to":"AAAAP3%2F%2F%2FLc%3D","updated":"2013-07-08 08:34:47.000000000","message":"Done","commit_id":"5385a1e93067c5c0f5cb188aae71644d391b5992"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"fc3849b271ed7ebc77e724389a3f331d70445f80","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                image.status \u003d \u0027queued\u0027"},{"line_number":207,"context_line":"        else:   # NOTE(zhiyan): len(image.locations) \u003d\u003d 0"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                image.locations.extend(value)"},{"line_number":210,"context_line":"                if image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":211,"context_line":"                    image.status \u003d \u0027active\u0027"},{"line_number":212,"context_line":"            except exception.BadStoreUri as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F7ZI%3D","line":209,"updated":"2013-07-09 02:08:42.000000000","message":"given the constraint that image.locations \u003d\u003d 0, extend should work. However, I think the meaning of replace is more accurately reflected in\n\n  image.locations \u003d value\n\nThat would work wouldn\u0027t it? Would you mind changing to that?","commit_id":"e461316074415b84e9af7f23011ae2d16a790a15"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"c91ea7e47b7ad26122d058d3457fd45e2a11aa06","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                image.status \u003d \u0027queued\u0027"},{"line_number":207,"context_line":"        else:   # NOTE(zhiyan): len(image.locations) \u003d\u003d 0"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                image.locations.extend(value)"},{"line_number":210,"context_line":"                if image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":211,"context_line":"                    image.status \u003d \u0027active\u0027"},{"line_number":212,"context_line":"            except exception.BadStoreUri as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F7O0%3D","line":209,"in_reply_to":"AAAAP3%2F%2F7ZI%3D","updated":"2013-07-09 03:59:19.000000000","message":"Done","commit_id":"e461316074415b84e9af7f23011ae2d16a790a15"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"fc3849b271ed7ebc77e724389a3f331d70445f80","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"},{"line_number":221,"context_line":"                                         len(image.locations), True)"},{"line_number":222,"context_line":"        if pos is None:"},{"line_number":223,"context_line":"            msg \u003d _(\"Invalid position to adding the location.\")"},{"line_number":224,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":225,"context_line":"        try:"},{"line_number":226,"context_line":"            image.locations.insert(pos, value)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F7Y0%3D","line":223,"updated":"2013-07-09 02:08:42.000000000","message":"Would you mind the following changes?\n\ns/to/for/\ns/the/a/","commit_id":"e461316074415b84e9af7f23011ae2d16a790a15"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"c91ea7e47b7ad26122d058d3457fd45e2a11aa06","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"},{"line_number":221,"context_line":"                                         len(image.locations), True)"},{"line_number":222,"context_line":"        if pos is None:"},{"line_number":223,"context_line":"            msg \u003d _(\"Invalid position to adding the location.\")"},{"line_number":224,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":225,"context_line":"        try:"},{"line_number":226,"context_line":"            image.locations.insert(pos, value)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F7Os%3D","line":223,"in_reply_to":"AAAAP3%2F%2F7Y0%3D","updated":"2013-07-09 03:59:19.000000000","message":"Done","commit_id":"e461316074415b84e9af7f23011ae2d16a790a15"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"fc3849b271ed7ebc77e724389a3f331d70445f80","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"},{"line_number":236,"context_line":"                                         len(image.locations), False)"},{"line_number":237,"context_line":"        if pos is None:"},{"line_number":238,"context_line":"            msg \u003d _(\"Invalid position to removing the location.\")"},{"line_number":239,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":240,"context_line":"        try:"},{"line_number":241,"context_line":"            # NOTE(zhiyan): this actually deletes the location"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F7Yw%3D","line":238,"updated":"2013-07-09 02:08:42.000000000","message":"Same substitutions here, please.","commit_id":"e461316074415b84e9af7f23011ae2d16a790a15"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"c91ea7e47b7ad26122d058d3457fd45e2a11aa06","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"},{"line_number":236,"context_line":"                                         len(image.locations), False)"},{"line_number":237,"context_line":"        if pos is None:"},{"line_number":238,"context_line":"            msg \u003d _(\"Invalid position to removing the location.\")"},{"line_number":239,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":240,"context_line":"        try:"},{"line_number":241,"context_line":"            # NOTE(zhiyan): this actually deletes the location"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F7N8%3D","line":238,"in_reply_to":"AAAAP3%2F%2F7Yw%3D","updated":"2013-07-09 03:59:19.000000000","message":"Done","commit_id":"e461316074415b84e9af7f23011ae2d16a790a15"},{"author":{"_account_id":6493,"name":"John Bresnahan","email":"john@bresnahan.me","username":"jbresnah"},"change_message_id":"4e662febd0094decb630a8fd7688a2ddf27d18bf","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            # from the backend store."},{"line_number":204,"context_line":"            del image.locations[:]"},{"line_number":205,"context_line":"            if image.status \u003d\u003d \u0027active\u0027:"},{"line_number":206,"context_line":"                image.status \u003d \u0027queued\u0027"},{"line_number":207,"context_line":"        else:   # NOTE(zhiyan): len(image.locations) \u003d\u003d 0"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                image.locations \u003d value"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2F1Sw%3D","line":206,"updated":"2013-07-10 18:15:51.000000000","message":"The transition of states in this way makes me nervous.  I prefer to see a single place where states transition based on the input of an event.  For example in this case the event would be a \u0027location_delete\u0027.  That would be sent into the image state_machine where the next state would be determined based on that event and the current state.\n\nI recognize that this would be a change to glance that would be quite pervasive and probably not good as part of this patch, but perhaps this patch could have a single method that does a similar thing.  I am concerned that some state transitions will be missed (or at least not obvious).  For example what happens if the status is \u0027error\u0027 here?  If the error is based on an upload problem, eliminating the location may clear that and moving it to queued may be the right transition.  I am also not sure what should happen in the pending_delete etc.","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"c5155571fe936125212a944ac922ac9732fc43d8","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            # from the backend store."},{"line_number":204,"context_line":"            del image.locations[:]"},{"line_number":205,"context_line":"            if image.status \u003d\u003d \u0027active\u0027:"},{"line_number":206,"context_line":"                image.status \u003d \u0027queued\u0027"},{"line_number":207,"context_line":"        else:   # NOTE(zhiyan): len(image.locations) \u003d\u003d 0"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                image.locations \u003d value"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2FzzA%3D","line":206,"in_reply_to":"AAAAP3%2F%2F02k%3D","updated":"2013-07-11 07:20:19.000000000","message":"After thinking, IMO there have 2 ways to resolve it:\n1. Adding a checking in this branch, only allow client remove locations to the image which under \u0027active\u0027 status.\n2. I think currently we just check \u0027active\u0027 in here is OK. My reason is following. We know image status value can be \u0027active\u0027, \u0027saving\u0027, \u0027queued\u0027, \u0027killed\u0027, \u0027pending_delete\u0027, \u0027deleted\u0027. And for \u0027queued\u0027, \u0027killed\u0027, \u0027pending_delete\u0027, \u0027deleted\u0027, I think you have no concerns right, and line #205 does not touch them, so probably you just concerned on \u0027saving\u0027 status here. After I check code, I found there are 2 points in code related to \u0027saving\u0027 status [1] and [2], if you check there details, you can see the image status be marked to \u0027saving\u0027 and begin to save data to store, and AFTER that, the new locations will be added to image locations property and update its status to \u0027active\u0027, so in this context, this PATCH handler removing all locations operation here will not affect that image saving progress, since the new location which saving now has not be added to image\u0027s locations property yet, so #204 will not affect that, and under #205 checking, the image will stay on \u0027saving\u0027 status, and image after the saving progress, the image will change to \u0027acitve\u0027 with one location record.\n\nI know maybe above #1 way is better then #1, because:\na. that\u0027s more easier and clear to code and client.\nb. the image status changing does not make sense to client a little, since when client ask to replace locations to empty, they will hope image status change to \u0027queued\u0027. But if we use above #2 way, that means client will see \u0027saving\u0027 status stay there even client replace the locations to empty, and image will change to \u0027active\u0027 later and it have one location record.\n\nWhich one you prefer? jbresnahan.\n\n[1] v1.images.Controller._activate() (https://github.com/openstack/glance/blob/master/glance/api/v1/images.py#L467) \n\n[2] v2.image_data.ImageDatacontroller.upload() (https://github.com/openstack/glance/blob/master/glance/api/v2/image_data.py#L53, https://github.com/openstack/glance/blob/master/glance/store/__init__.py#L392)","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6493,"name":"John Bresnahan","email":"john@bresnahan.me","username":"jbresnah"},"change_message_id":"483871ae6221354fb5febb74afd6d97dc5eb217f","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            # from the backend store."},{"line_number":204,"context_line":"            del image.locations[:]"},{"line_number":205,"context_line":"            if image.status \u003d\u003d \u0027active\u0027:"},{"line_number":206,"context_line":"                image.status \u003d \u0027queued\u0027"},{"line_number":207,"context_line":"        else:   # NOTE(zhiyan): len(image.locations) \u003d\u003d 0"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                image.locations \u003d value"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2F02k%3D","line":206,"in_reply_to":"AAAAP3%2F%2F1FY%3D","updated":"2013-07-10 20:40:47.000000000","message":"A new BP sounds good for that.  However, in this case I would still like a better idea of what happens when this event occurs and the image is on states other than active","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"f2050ab810b2d236db323d7c56d0834be7303bb6","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            # from the backend store."},{"line_number":204,"context_line":"            del image.locations[:]"},{"line_number":205,"context_line":"            if image.status \u003d\u003d \u0027active\u0027:"},{"line_number":206,"context_line":"                image.status \u003d \u0027queued\u0027"},{"line_number":207,"context_line":"        else:   # NOTE(zhiyan): len(image.locations) \u003d\u003d 0"},{"line_number":208,"context_line":"            try:"},{"line_number":209,"context_line":"                image.locations \u003d value"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2F1FY%3D","line":206,"in_reply_to":"AAAAP3%2F%2F1Sw%3D","updated":"2013-07-10 19:41:14.000000000","message":"Overall I think state_machine requirement is a valuable things for Glance. But it seems little like overkill to this patch. IMO, I don\u0027t think we can resolve the whole problem in this patch one time. I can refactoring the code once state_machine ready. We can create a dedicate BP cover that.","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6493,"name":"John Bresnahan","email":"john@bresnahan.me","username":"jbresnah"},"change_message_id":"4e662febd0094decb630a8fd7688a2ddf27d18bf","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        except exception.BadStoreUri as e:"},{"line_number":230,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":231,"context_line":"        except ValueError:"},{"line_number":232,"context_line":"            pass    # update image status failed."},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _do_remove_locations(self, image, path_pos):"},{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2F1To%3D","line":232,"updated":"2013-07-10 18:15:51.000000000","message":"By looking glance/domain/__init__.py:96 I see why this ValueError is trapped.  However I am wondering about this case:\n\nA user creates an image but does not upload it and does not provide a disk-format nor a container-format.  This leaves the image in the \u0027queued\u0027 state.  The users intention was never to upload data but rather to associate with that data a location.  If they do so with this method a ValueError will be raised, but ignored.  This will leave the image in the \u0027queued\u0027 state and thus make the image unusable without giving the user an indication as to why.\n\nPerhaps we should not allow locations to be added unless the image is ready to translate to the active state (or is in the active or saving state).","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"f2050ab810b2d236db323d7c56d0834be7303bb6","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        except exception.BadStoreUri as e:"},{"line_number":230,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":231,"context_line":"        except ValueError:"},{"line_number":232,"context_line":"            pass    # update image status failed."},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _do_remove_locations(self, image, path_pos):"},{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2F1HM%3D","line":232,"in_reply_to":"AAAAP3%2F%2F1To%3D","updated":"2013-07-10 19:41:14.000000000","message":"Done","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6493,"name":"John Bresnahan","email":"john@bresnahan.me","username":"jbresnah"},"change_message_id":"4e662febd0094decb630a8fd7688a2ddf27d18bf","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        except Exception as e:"},{"line_number":245,"context_line":"            raise webob.exc.HTTPInternalServerError(explanation\u003dunicode(e))"},{"line_number":246,"context_line":"        if (len(image.locations) \u003d\u003d 0) and (image.status \u003d\u003d \u0027active\u0027):"},{"line_number":247,"context_line":"            image.status \u003d \u0027queued\u0027"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"class RequestDeserializer(wsgi.JSONRequestDeserializer):"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2F1Qg%3D","line":247,"updated":"2013-07-10 18:15:51.000000000","message":"What if the status was saving and we just popped out the location to which it was saving?","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"f2050ab810b2d236db323d7c56d0834be7303bb6","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        except Exception as e:"},{"line_number":245,"context_line":"            raise webob.exc.HTTPInternalServerError(explanation\u003dunicode(e))"},{"line_number":246,"context_line":"        if (len(image.locations) \u003d\u003d 0) and (image.status \u003d\u003d \u0027active\u0027):"},{"line_number":247,"context_line":"            image.status \u003d \u0027queued\u0027"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"class RequestDeserializer(wsgi.JSONRequestDeserializer):"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAP3%2F%2F1H4%3D","line":247,"in_reply_to":"AAAAP3%2F%2F1Qg%3D","updated":"2013-07-10 19:41:14.000000000","message":"Done","commit_id":"178f6a5ad8871cc66ed1372e4bd4e1183e0faf64"},{"author":{"_account_id":6493,"name":"John Bresnahan","email":"john@bresnahan.me","username":"jbresnah"},"change_message_id":"abe293daf3b041c84bbba5ac94f9e02878d06c96","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                pass    # update image status failed."},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def _do_add_locations(self, image, path_pos, value):"},{"line_number":218,"context_line":"        if image.status not in [\u0027active\u0027, \u0027saving\u0027]:"},{"line_number":219,"context_line":"            msg \u003d _(\"Only allow adding a location to an image \""},{"line_number":220,"context_line":"                    \"which under \u0027active\u0027 or \u0027saving\u0027 status.\")"},{"line_number":221,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAP3%2F%2F004%3D","line":218,"updated":"2013-07-10 20:48:28.000000000","message":"This check probably needs to be in replace_locations in the branch where images are added as well.","commit_id":"d80c6c565180c058f91293796b119689f10d0dac"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"b5a78df4cc9a335fc5d27242b51bf3c36a9e713f","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                pass    # update image status failed."},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def _do_add_locations(self, image, path_pos, value):"},{"line_number":218,"context_line":"        if image.status not in [\u0027active\u0027, \u0027saving\u0027]:"},{"line_number":219,"context_line":"            msg \u003d _(\"Only allow adding a location to an image \""},{"line_number":220,"context_line":"                    \"which under \u0027active\u0027 or \u0027saving\u0027 status.\")"},{"line_number":221,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAP3%2F%2Fz4Y%3D","line":218,"in_reply_to":"AAAAP3%2F%2F004%3D","updated":"2013-07-11 07:21:22.000000000","message":"Done","commit_id":"d80c6c565180c058f91293796b119689f10d0dac"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"3588fa537e08699ad9b3e9ca68eb155ecfd473d8","unresolved":false,"context_lines":[{"line_number":212,"context_line":"            except exception.BadStoreUri as e:"},{"line_number":213,"context_line":"                raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":214,"context_line":"            except ValueError:"},{"line_number":215,"context_line":"                pass    # update image status failed."},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def _do_add_locations(self, image, path_pos, value):"},{"line_number":218,"context_line":"        # NOTE(zhiyan): Don\u0027t allow client add a list in one call"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAP3%2F%2FtE8%3D","line":215,"updated":"2013-07-12 18:06:34.000000000","message":"I believe we do not want to ignore this exception. This happens when someone adds a location to a queued image, but the container or disk format is not set. Normally, when users try to upload data, we require that they have already set those fields, and I think we should stay consistent with that behavior.\n\nSo this should be amended to\n\n  except ValueError as e:\n      raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))","commit_id":"d4ae54c4d1c5881913e78f4b207bc2ea800ffe2e"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"3588fa537e08699ad9b3e9ca68eb155ecfd473d8","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        except exception.BadStoreUri as e:"},{"line_number":230,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":231,"context_line":"        except ValueError:"},{"line_number":232,"context_line":"            pass    # update image status failed."},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _do_remove_locations(self, image, path_pos):"},{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAP3%2F%2FtE0%3D","line":232,"updated":"2013-07-12 18:06:34.000000000","message":"Same note is true here.","commit_id":"d4ae54c4d1c5881913e78f4b207bc2ea800ffe2e"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"6866e17b68779221eefe274d76b6dd2f110aa56c","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        except exception.BadStoreUri as e:"},{"line_number":230,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":231,"context_line":"        except ValueError:"},{"line_number":232,"context_line":"            pass    # update image status failed."},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _do_remove_locations(self, image, path_pos):"},{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAP3%2F%2Fs5s%3D","line":232,"in_reply_to":"AAAAP3%2F%2Fs%2FI%3D","updated":"2013-07-12 18:58:54.000000000","message":"John,\n\nI actually asked Zhi to take out those checks, because I think\n\n1) we should support the case of adding a location to an image in the \u0027queued\u0027 state to bring it to \u0027active\u0027\n\n2) we don\u0027t really have to worry much about the image in the saving state, since locations are added as the image goes to \u0027active\u0027","commit_id":"d4ae54c4d1c5881913e78f4b207bc2ea800ffe2e"},{"author":{"_account_id":6493,"name":"John Bresnahan","email":"john@bresnahan.me","username":"jbresnah"},"change_message_id":"a03aa461970e1c10fb7297c27be7460d7e8e35ae","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        except exception.BadStoreUri as e:"},{"line_number":230,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":231,"context_line":"        except ValueError:"},{"line_number":232,"context_line":"            pass    # update image status failed."},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _do_remove_locations(self, image, path_pos):"},{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAP3%2F%2Fs28%3D","line":232,"in_reply_to":"AAAAP3%2F%2Fs5s%3D","updated":"2013-07-12 19:12:51.000000000","message":"Thanks Mark, that makes sense.\n\nAll of the state management her does still make me nervous.  When this is not fresh in peoples minds it may be hard mentally reconstruct the state machine.  That said I think that fix is beyond this patch.","commit_id":"d4ae54c4d1c5881913e78f4b207bc2ea800ffe2e"},{"author":{"_account_id":6493,"name":"John Bresnahan","email":"john@bresnahan.me","username":"jbresnah"},"change_message_id":"8ac19633809e77e01c89f3f09e248ee8eb7483e9","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        except exception.BadStoreUri as e:"},{"line_number":230,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":231,"context_line":"        except ValueError:"},{"line_number":232,"context_line":"            pass    # update image status failed."},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _do_remove_locations(self, image, path_pos):"},{"line_number":235,"context_line":"        pos \u003d self._get_locations_op_pos(path_pos,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAP3%2F%2Fs%2FI%3D","line":232,"in_reply_to":"AAAAP3%2F%2FtE0%3D","updated":"2013-07-12 18:32:44.000000000","message":"Mark: Zhi and I talked about that a bit.  In PS11 there was a  check for state at the beginning to resolve this:\n\nhttps://review.openstack.org/#/c/35134/11/glance/api/v2/images.py (zhi what made you take that out?)\n\nHowever, I prefer bubbling up the error as you mention here","commit_id":"d4ae54c4d1c5881913e78f4b207bc2ea800ffe2e"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"f3f17a378c860b6f85cc6e823ffdbd5ed4bcc273","unresolved":false,"context_lines":[{"line_number":691,"context_line":"                    \u0027type\u0027: \u0027object\u0027,"},{"line_number":692,"context_line":"                },"},{"line_number":693,"context_line":"            },"},{"line_number":694,"context_line":"            \u0027required\u0027: [\u0027url\u0027, \u0027metadata\u0027],"},{"line_number":695,"context_line":"        },"},{"line_number":696,"context_line":"        \u0027description\u0027: _(\u0027A set of URLs to access the image file kept in \u0027"},{"line_number":697,"context_line":"                         \u0027external store\u0027),"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAP3%2F%2FrTI%3D","line":694,"updated":"2013-07-13 21:04:17.000000000","message":"Is this the right way to express required-ness? In http://tools.ietf.org/html/draft-zyp-json-schema-03#section-5.7 it seems like you would want\n\n  \u0027items\u0027: {\n    \u0027type\u0027: \u0027object\u0027,\n    \u0027properties\u0027: {\n      \u0027url\u0027: {\n        \u0027type\u0027: \u0027string\u0027,\n        \u0027maxLength\u0027: 255,\n        \u0027required\u0027: True\n      },\n      \u0027metadata\u0027: {\n        \u0027type\u0027: \u0027object\u0027,\n        \u0027required\u0027: True\n      }\n    }\n  }\n\n\nAs an additional note, why do we want to specify that these parameters are required? We do not specify other attributres as required even though they are in an equivalent position.\n\nMy suggestion is we should not be adding \"required\" in any form at this time.","commit_id":"1e40cf967f671392d79f5368ad277a6a373d7040"},{"author":{"_account_id":616,"name":"Mark Washenberger","email":"mark.washenberger@markwash.net","username":"markwash"},"change_message_id":"5a5b6db02279aac9896f42112d0d5764c94969c8","unresolved":false,"context_lines":[{"line_number":691,"context_line":"                    \u0027type\u0027: \u0027object\u0027,"},{"line_number":692,"context_line":"                },"},{"line_number":693,"context_line":"            },"},{"line_number":694,"context_line":"            \u0027required\u0027: [\u0027url\u0027, \u0027metadata\u0027],"},{"line_number":695,"context_line":"        },"},{"line_number":696,"context_line":"        \u0027description\u0027: _(\u0027A set of URLs to access the image file kept in \u0027"},{"line_number":697,"context_line":"                         \u0027external store\u0027),"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAP3%2F%2FrNk%3D","line":694,"in_reply_to":"AAAAP3%2F%2FrTI%3D","updated":"2013-07-14 00:21:51.000000000","message":"Alas, I\u0027m incorrect about how \"required\" is used currently in jsonschema tools. I believe your usage is consistent with the tools we are using to validate images.\n\nGood catch on using the schema validation to make the metadata and url parts of an added location work!","commit_id":"1e40cf967f671392d79f5368ad277a6a373d7040"}]}