)]}'
{"glanceclient/v2/images.py":[{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"558b1d0fe6f72507e73f2d1d98291d26d91acd04","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":82,"context_line":"        return sort"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def list(self, request_ids\u003dNone, **kwargs):"},{"line_number":85,"context_line":"        \"\"\"Retrieve a listing of Image objects."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        :param page_size: Number of images to request in each"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_01d4e71d","line":84,"range":{"start_line":84,"start_character":19,"end_line":84,"end_character":30},"updated":"2015-12-24 15:24:02.000000000","message":"Is it reasonable to implement general solution? I mean can\u0027t we pass hook as callable object to the methods. users then can extract any attributes that they want from this object.\nthe interface would be like:\n# some object that \nreq_id_list \u003d []\nhook \u003d AppendHeaderHook(\u0027x-openstack-request-id\u0027, req_id_list)\nglanceclient.list(hooks \u003d [hook])\n# usage of req_id_list\n\nThen we don\u0027t need to manage the logic of managing of request_id at all.\nThe interface also would be more clear to the users.\nAlso once smbd want to do the same but for different header we don\u0027t need to re-implement solution. Just pass the different hook.","commit_id":"c1e993b57a002e77c4077dc90378b21c0ad4a5f7"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"1ae31221d273337f26bd8de008cf6ccdf3ec3d39","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":82,"context_line":"        return sort"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def list(self, request_ids\u003dNone, **kwargs):"},{"line_number":85,"context_line":"        \"\"\"Retrieve a listing of Image objects."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        :param page_size: Number of images to request in each"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_e170bbde","line":84,"range":{"start_line":84,"start_character":19,"end_line":84,"end_character":30},"in_reply_to":"1a7b0d38_01d4e71d","updated":"2015-12-24 15:35:45.000000000","message":"Also I was wondering about security for this approach but it seems that security will be ok if we restrict this to response hook only.","commit_id":"c1e993b57a002e77c4077dc90378b21c0ad4a5f7"},{"author":{"_account_id":18494,"name":"Ravi Shekhar Jethani","email":"rsjethani@gmail.com","username":"rsjethani"},"change_message_id":"e46a0a2365ad3e9afccb8164885350188a9b8bf7","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":82,"context_line":"        return sort"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def list(self, request_ids\u003dNone, **kwargs):"},{"line_number":85,"context_line":"        \"\"\"Retrieve a listing of Image objects."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        :param page_size: Number of images to request in each"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_9460bcd0","line":84,"range":{"start_line":84,"start_character":19,"end_line":84,"end_character":30},"in_reply_to":"1a7b0d38_01d4e71d","updated":"2015-12-29 11:13:52.000000000","message":"The approach you are suggesting looks good but it depends too much on the internals of the client API. Some examples:\n* The \u0027AppendHeaderHook\u0027 method has to know that request id is present as a value for a key called \u0027x-openstack-request-id\u0027. What if in future this key changes?\n* The \u0027AppendHeaderHook\u0027 method is assuming that it will be given a \u0027Response\u0027 object but the API may decide to use something else in the future.\n\nThe current approach is more robust from this point of view. In the current approach the API says \"give me an empty list and I will fill it with request ids\". How the API extracts request ids is its internal matter(which the API is free to change). IMO this is good OOP.","commit_id":"c1e993b57a002e77c4077dc90378b21c0ad4a5f7"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"558b1d0fe6f72507e73f2d1d98291d26d91acd04","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        body.pop(\u0027self\u0027, None)"},{"line_number":187,"context_line":"        return self.model(**body)"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    def data(self, image_id, do_checksum\u003dTrue, request_ids\u003dNone):"},{"line_number":190,"context_line":"        \"\"\"Retrieve data of an image."},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        :param image_id:    ID of the image to download."}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_4ac5298c","line":189,"range":{"start_line":189,"start_character":47,"end_line":189,"end_character":58},"updated":"2015-12-24 15:24:02.000000000","message":"Could you please add doc string for the new parameter?","commit_id":"c1e993b57a002e77c4077dc90378b21c0ad4a5f7"},{"author":{"_account_id":18494,"name":"Ravi Shekhar Jethani","email":"rsjethani@gmail.com","username":"rsjethani"},"change_message_id":"e46a0a2365ad3e9afccb8164885350188a9b8bf7","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        body.pop(\u0027self\u0027, None)"},{"line_number":187,"context_line":"        return self.model(**body)"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    def data(self, image_id, do_checksum\u003dTrue, request_ids\u003dNone):"},{"line_number":190,"context_line":"        \"\"\"Retrieve data of an image."},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        :param image_id:    ID of the image to download."}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_d4b16448","line":189,"range":{"start_line":189,"start_character":47,"end_line":189,"end_character":58},"in_reply_to":"1a7b0d38_4ac5298c","updated":"2015-12-29 11:13:52.000000000","message":"Done","commit_id":"c1e993b57a002e77c4077dc90378b21c0ad4a5f7"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"558b1d0fe6f72507e73f2d1d98291d26d91acd04","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        \"\"\""},{"line_number":196,"context_line":"        hook \u003d utils.setup_request_id_hook(request_ids)"},{"line_number":197,"context_line":"        url \u003d \u0027/v2/images/%s/file\u0027 % image_id"},{"line_number":198,"context_line":"        resp, body \u003d self.http_client.get(url, hooks\u003dhook)"},{"line_number":199,"context_line":"        if resp.status_code \u003d\u003d codes.no_content:"},{"line_number":200,"context_line":"            return None"},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_c109ff2d","line":198,"range":{"start_line":198,"start_character":47,"end_line":198,"end_character":52},"updated":"2015-12-24 15:24:02.000000000","message":"I guess we are using hooks here because it allows to ensure that request_id is generated even if status \u003e 400.\nWould it be reasonable to explain it somewhere in comments?","commit_id":"c1e993b57a002e77c4077dc90378b21c0ad4a5f7"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"d5245290c6305733dceacb20e76afa87f87f60f8","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    @utils.memoized_property"},{"line_number":40,"context_line":"    def model(self):"},{"line_number":41,"context_line":"        schema \u003d self.schema_client.get(\u0027image\u0027)"},{"line_number":42,"context_line":"        warlock_model \u003d warlock.model_factory(schema.raw(),"},{"line_number":43,"context_line":"                                              schemas.SchemaBasedModel)"},{"line_number":44,"context_line":"        return warlock_model"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a5ae5dd_c0c2d252","line":41,"range":{"start_line":41,"start_character":22,"end_line":41,"end_character":35},"updated":"2016-02-08 10:09:44.000000000","message":"Questions: are we going make request_ids for schema requests?\nIf not, why\u003d)?","commit_id":"ba4520e74429a6634cad0791418ce29fcdbbf037"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"a810145db664841c84362e29bf5dba5c84a8f18f","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    @utils.memoized_property"},{"line_number":40,"context_line":"    def model(self):"},{"line_number":41,"context_line":"        schema \u003d self.schema_client.get(\u0027image\u0027)"},{"line_number":42,"context_line":"        warlock_model \u003d warlock.model_factory(schema.raw(),"},{"line_number":43,"context_line":"                                              schemas.SchemaBasedModel)"},{"line_number":44,"context_line":"        return warlock_model"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a57f1b5_3004cc48","line":41,"range":{"start_line":41,"start_character":22,"end_line":41,"end_character":35},"in_reply_to":"5a5ae5dd_4df1fb94","updated":"2016-02-11 07:55:06.000000000","message":"Ah, ok. Makes sens","commit_id":"ba4520e74429a6634cad0791418ce29fcdbbf037"},{"author":{"_account_id":18494,"name":"Ravi Shekhar Jethani","email":"rsjethani@gmail.com","username":"rsjethani"},"change_message_id":"242c32b1673d4b7d79726923ee03d2eccd3cb713","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    @utils.memoized_property"},{"line_number":40,"context_line":"    def model(self):"},{"line_number":41,"context_line":"        schema \u003d self.schema_client.get(\u0027image\u0027)"},{"line_number":42,"context_line":"        warlock_model \u003d warlock.model_factory(schema.raw(),"},{"line_number":43,"context_line":"                                              schemas.SchemaBasedModel)"},{"line_number":44,"context_line":"        return warlock_model"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a5ae5dd_4df1fb94","line":41,"range":{"start_line":41,"start_character":22,"end_line":41,"end_character":35},"in_reply_to":"5a5ae5dd_c0c2d252","updated":"2016-02-09 07:42:10.000000000","message":"\u0027@utils.memoized_property\u0027 decotator turns model() into a getter property. Because of this we cannot pass \u0027request_ids\u0027 list to model(). As a result we cannot modify this list to contain request id obtained while fetching the schema.\n\nWe may try to return request ids along with the warlock_model but again this is not possible because of the memoized_property decorator.\n\nNOTE: Schema is fetched lazily i.e. a schema is fetched only when it is first required and subsequent calls to get the schema do not actually make a glance request but simply\nreturn a local copy of the schema.","commit_id":"ba4520e74429a6634cad0791418ce29fcdbbf037"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"d5245290c6305733dceacb20e76afa87f87f60f8","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":82,"context_line":"        return sort"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def list(self, request_ids\u003dNone, **kwargs):"},{"line_number":85,"context_line":"        \"\"\"Retrieve a listing of Image objects."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        :param page_size: Number of images to request in each"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a5ae5dd_40a7c20a","line":84,"updated":"2016-02-08 10:09:44.000000000","message":"Could you please modify docstrings cause it is important for API methods, IMO.","commit_id":"ba4520e74429a6634cad0791418ce29fcdbbf037"},{"author":{"_account_id":18494,"name":"Ravi Shekhar Jethani","email":"rsjethani@gmail.com","username":"rsjethani"},"change_message_id":"8985ebb84156d9ed9f576bc172128f9f67c53f6d","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":82,"context_line":"        return sort"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def list(self, request_ids\u003dNone, **kwargs):"},{"line_number":85,"context_line":"        \"\"\"Retrieve a listing of Image objects."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        :param page_size: Number of images to request in each"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a5ae5dd_955139db","line":84,"in_reply_to":"5a5ae5dd_40a7c20a","updated":"2016-02-09 09:38:29.000000000","message":"Done","commit_id":"ba4520e74429a6634cad0791418ce29fcdbbf037"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"d5245290c6305733dceacb20e76afa87f87f60f8","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        # it could be possible to send invalid data to the server by passing"},{"line_number":95,"context_line":"        # page_size\u003dNone."},{"line_number":96,"context_line":"        page_size \u003d kwargs.get(\u0027page_size\u0027) or DEFAULT_PAGE_SIZE"},{"line_number":97,"context_line":"        hook \u003d utils.setup_request_id_hook(request_ids)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        def paginate(url, page_size, limit\u003dNone):"},{"line_number":100,"context_line":"            next_url \u003d url"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a5ae5dd_603afe9a","line":97,"updated":"2016-02-08 10:09:44.000000000","message":"I am still doubt if we need to pass hooks directly in order to be extensible here but after additional analysis looks like request_ids is more proper.","commit_id":"ba4520e74429a6634cad0791418ce29fcdbbf037"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"43c100b724bb3bd48c4f3cc7c8877dbd6d6a472b","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        :param image_id: ID of the image to upload data for."},{"line_number":217,"context_line":"        :param image_data: File-like object supplying the data to upload."},{"line_number":218,"context_line":"        :param image_size: Unused - present for backwards compatability"},{"line_number":219,"context_line":"        \"\"\""},{"line_number":220,"context_line":"        hook \u003d utils.setup_request_id_hook(request_ids)"},{"line_number":221,"context_line":"        url \u003d \u0027/v2/images/%s/file\u0027 % image_id"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a57f1b5_902a602b","line":218,"updated":"2016-02-11 07:53:57.000000000","message":"Could we add doc string for other methods?\nIMO, it would help client users to understand what is request_ids","commit_id":"8760a0c0a4e5730bd59099b7f02cbd3f73863ff0"},{"author":{"_account_id":455,"name":"Stuart McLaren","email":"stuart.mclaren@hpe.com","username":"stuart-mclaren"},"change_message_id":"3ba20ca54757a854d3628fea453c83da1b8574f5","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # it could be possible to send invalid data to the server by passing"},{"line_number":97,"context_line":"        # page_size\u003dNone."},{"line_number":98,"context_line":"        page_size \u003d kwargs.get(\u0027page_size\u0027) or DEFAULT_PAGE_SIZE"},{"line_number":99,"context_line":"        hook \u003d utils.setup_request_id_hook(request_ids)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        def paginate(url, page_size, limit\u003dNone):"},{"line_number":102,"context_line":"            next_url \u003d url"}],"source_content_type":"text/x-python","patch_set":6,"id":"dae33548_8780788c","line":99,"range":{"start_line":99,"start_character":8,"end_line":99,"end_character":12},"updated":"2016-02-18 19:20:48.000000000","message":"Is it possible to move hook to the top of the function for each case?\n\n\nI wonder if it would be possible and worth it to look at using decorators, eg:\n\n\n def request_id(func, *args, **kwargs):\n    def wrapped_fn(self, *args, **kwargs):\n        kwargs[\u0027hook\u0027] \u003d utils.setup_request_id_hook(kwargs.pop(\u0027request_ids\u0027))\n        return func(self, *args, **kwargs)\n    return wrapped_fn\n\n\n    @request_id\n    def get(self, image_id, **kwargs):\n        url \u003d \u0027/v2/images/%s\u0027 % image_id\n        resp, body \u003d self.http_client.get(url, hooks\u003dkwargs[\u0027hook\u0027])\n        # NOTE(bcwaldon): remove \u0027self\u0027 for now until we have an elegant\n        # way to pass it into the model constructor without conflict\n        body.pop(\u0027self\u0027, None)\n        return self.unvalidated_model(**body)\n\nI\u0027m actually not sure, but thought I\u0027d throw the idea out there.","commit_id":"8e1d12a3f780b439c6065db0fc467f7452341d8b"},{"author":{"_account_id":18494,"name":"Ravi Shekhar Jethani","email":"rsjethani@gmail.com","username":"rsjethani"},"change_message_id":"a0c6c194818232cd84996bb90fbe88f18e6cec44","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # it could be possible to send invalid data to the server by passing"},{"line_number":97,"context_line":"        # page_size\u003dNone."},{"line_number":98,"context_line":"        page_size \u003d kwargs.get(\u0027page_size\u0027) or DEFAULT_PAGE_SIZE"},{"line_number":99,"context_line":"        hook \u003d utils.setup_request_id_hook(request_ids)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        def paginate(url, page_size, limit\u003dNone):"},{"line_number":102,"context_line":"            next_url \u003d url"}],"source_content_type":"text/x-python","patch_set":6,"id":"bae84128_da0405c8","line":99,"range":{"start_line":99,"start_character":8,"end_line":99,"end_character":12},"in_reply_to":"dae33548_8780788c","updated":"2016-02-19 10:36:09.000000000","message":"Hi Stuart,\n\nThanks for your review.\n\nActually using decorator was our first choice but as Cao explained the decorator will work with cases like get() but will not work for create() and update() for example.\n\nThe kwargs in update() and create() can only contain keys allowed by the schema. \u0027request_ids\u0027 and \u0027hook\u0027 do not belong to the image schema as a result we get schema validation\nerrors:\n\n\u003e\u003e\u003e import glanceclient\n\u003e\u003e\u003e glance \u003d glanceclient.Client(\u00272\u0027, endpoint\u003d\u0027http://10.69.4.136:9292/\u0027, token\u003d\u00276234bed80b72489baf9114b894be68b9\u0027)\n\u003e\u003e\u003e ids \u003d []\n\u003e\u003e\u003e res \u003d glance.images.update(\u002795989bfb-9cc9-472e-bec4-31abf3b7df68\u0027, name\u003d\u0027newname\u0027, request_ids\u003dids)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"glanceclient/v2/images.py\", line 185, in wrapped_fn\n    return func(self, *args, **kwargs)\n  File \"glanceclient/v2/images.py\", line 307, in update\n    raise TypeError(encodeutils.exception_to_unicode(e))\nTypeError: Unable to set \u0027hook\u0027 to \u0027{\u0027response\u0027: \u003cfunctools.partial object at 0x7fbff5cf8208\u003e}\u0027. Reason: {\u0027response\u0027: \u003cfunctools.partial object at 0x7fbff5cf8208\u003e} is not of type u\u0027string\u0027\n\nFailed validating u\u0027type\u0027 in schema[u\u0027additionalProperties\u0027]:\n    {u\u0027type\u0027: u\u0027string\u0027}\n\nOn instance[\u0027hook\u0027]:\n    {\u0027response\u0027: \u003cfunctools.partial object at 0x7fbff5cf8208\u003e}\n\n\nWe can work around this problem by first popping off \u0027request_ids\u0027 and \u0027hook\u0027 from kwargs to local variables before creating model object but that will only complicate things rather than simplifying.","commit_id":"8e1d12a3f780b439c6065db0fc467f7452341d8b"},{"author":{"_account_id":20105,"name":"Cao ShuFeng","email":"caosf.fnst@cn.fujitsu.com","username":"caosf.fnst"},"change_message_id":"923ce50b618f80ef4ae6ee82c214af7f35c7fde9","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # it could be possible to send invalid data to the server by passing"},{"line_number":97,"context_line":"        # page_size\u003dNone."},{"line_number":98,"context_line":"        page_size \u003d kwargs.get(\u0027page_size\u0027) or DEFAULT_PAGE_SIZE"},{"line_number":99,"context_line":"        hook \u003d utils.setup_request_id_hook(request_ids)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        def paginate(url, page_size, limit\u003dNone):"},{"line_number":102,"context_line":"            next_url \u003d url"}],"source_content_type":"text/x-python","patch_set":6,"id":"dae33548_f512c008","line":99,"range":{"start_line":99,"start_character":8,"end_line":99,"end_character":12},"in_reply_to":"dae33548_8780788c","updated":"2016-02-19 01:52:58.000000000","message":"Hi,Stuart McLaren:\n\nThis is my concern\n1.This decorator doesn\u0027t work for the update() function in line 284.\nThe update() function calls get() function in line 293 but the \u0027request_ids\u0027 is poped by warpped_fn().\n2.This decorator solution also needs a lot of change in code.\n3.If we add this decorator only for get() function, such difference will make user confused.\n\nThanks","commit_id":"8e1d12a3f780b439c6065db0fc467f7452341d8b"}]}