)]}'
{"glance/api/glare/v0_1/glare.py":[{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"1403e323adcc70f8df92b95b4d340729d99cede8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9a30ddce_cf9a9a13","updated":"2017-03-13 16:38:34.000000000","message":"This won\u0027t be needed any more after Glare-ectomy. A rebase should do the trick.","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"}],"glance/api/v2/image_data.py":[{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"1403e323adcc70f8df92b95b4d340729d99cede8","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_8f71820e","line":280,"range":{"start_line":278,"start_character":8,"end_line":280,"end_character":43},"updated":"2017-03-13 16:38:34.000000000","message":"Ian, can you help me understand why this change is needed? The change you made to has_body method of JSONRequestDeserializer in wsgi.py should be sufficient isn\u0027t it?\n\nEssentially, what I mean is, between checking webob\u0027s ``is_body_readable`` attribute and the combination of valid-transfer encoding and probable HTTP methods that can have bodies should be enough, right?","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"6a19910102c8859e759b3971d0ce5ca51dafcc5f","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_78785890","line":280,"range":{"start_line":278,"start_character":8,"end_line":280,"end_character":43},"updated":"2017-02-02 18:16:23.000000000","message":"why do we have this here? \nSince we are calling \u0027get_content_type\u0027, it is taken care of in wsgi.py right? [0] where \u0027chunked\u0027 is defined in L990.\n\n[0] https://github.com/openstack/glance/blob/master/glance/common/wsgi.py#L999-L1002","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"979f7c15dd3833899a3e2a66d2ec0460b42bdfc6","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1ff146_222a177f","line":280,"range":{"start_line":278,"start_character":8,"end_line":280,"end_character":43},"in_reply_to":"3a1ff146_f6863e64","updated":"2017-03-16 00:30:25.000000000","message":"Hemanth, yes, having this conditional check here is not required. We need to remove this redundant check from here. In my previous comment, I had confused myself with the 2 AND, OR operators.\nIn wsgi.py, we either can set the check separately, cos of the behaviour of request.is_body_readable or proceed with what we just have now in wsgi.py.\nThe former would maybe make things clearer but I admit that there is no big difference.","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"b03deec825918bf5a17fa573feafdc84b8364b27","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_aa534338","line":280,"range":{"start_line":278,"start_character":8,"end_line":280,"end_character":43},"in_reply_to":"3a461143_78785890","updated":"2017-02-02 21:30:17.000000000","message":"Because WebOb 1.7.0 no longer considers a chunked transfer-encoding to be readable and so `request.is_body_readable` ends up being false which breaks the condition in the block you highlighted.","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"f6300cef75c7f0f546bc57ac0373110e2d390a5b","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a461143_d9edc78f","line":280,"range":{"start_line":278,"start_character":8,"end_line":280,"end_character":43},"in_reply_to":"3a461143_aa534338","updated":"2017-02-02 22:23:48.000000000","message":"How about fixing it there then? In the has_body() method.\nCos this check is already done there. The \"AND\" operator is playing spoilsport.\nHow about moving \u0027is_valid_encoding\u0027 to an elif there?\nIt does exactly what you are doing here (forcing request.is_body_readable to True). We might as well do it there instead of rechecking here.","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"c88764d6d9d19d21d87ce186615c3cc50d4904ee","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1ff146_f6863e64","line":280,"range":{"start_line":278,"start_character":8,"end_line":280,"end_character":43},"in_reply_to":"9a30ddce_45492b30","updated":"2017-03-14 19:41:28.000000000","message":"Let\u0027s walk through an example:\nConsider POST, obviously with a body, and Transfer-Encoding \u0027chunked\u0027.\n\nOur existing condition is [0] and proposed is [1].\nNow, what changed here is the behavior of ``request.is_body_readable``, which used to be ``True``. With the newer versions of Webob, it won\u0027t be true any more. This breaks [0]. \n\nNow let\u0027s examine [1].\n``is_valid_encoding`` remains unchanged with ``True``.\n``request.is_body_readable`` would be ``False`` now.\n``request.method.upper() in self.httpmethod_may_have_body`` would be ``True`` because ``POST`` is a member of the frozenset ``httpmethod_may_have_body``.\n\nSo, what we have here is ``True AND (False OR True)``, which should evaluate to ``True``. Thus, ``has_body`` returns ``True`` any way without the check here.\n\nUnless we use request.is_body_readable elsewhere (I don\u0027t see any other use), setting it here is probably not required. Maybe it\u0027s just the right thing to do. If so, like you mentioned, it could probably go in ``wsgi.py`` module. The only difference being, the check in ``wsgi.py`` would make this behavior available for all the deserializers and not just the image data deserializer. But, that\u0027s probably okay. Right?\n\nMaybe I\u0027m digressing a bit but essentially my question was, why set ``request.is_body_readable`` here when what we have in ``wsgi.py`` seems to be sufficient? Not sure what I\u0027m missing here.\n\n\n[0] is_valid_encoding and request.is_body_readable\n[1] is_valid_encoding and (request.is_body_readable or request.method.upper() in self.httpmethod_may_have_body)","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d3ad0ab36e22ef00bdd14fcc55970833ef540132","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_45492b30","line":280,"range":{"start_line":278,"start_character":8,"end_line":280,"end_character":43},"in_reply_to":"9a30ddce_8f71820e","updated":"2017-03-13 17:16:08.000000000","message":"Hemanth, what Ian was telling is that in wsgi.py, \"request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027)\" is not being considered by webob 1.7. Hence we end up getting a False from there. This is because we do an AND of is_body_readable and is_valid_encoding.\n\nSo the check of transfer encoding header in wsgi.py becomes pointless.\nHence I suggested that we go with separate conditional checks in wsgi.py instead of repeating it here.","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"9132d8f8954751066ffe502fc41d08903df9904b","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if (request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower() \u003d\u003d"},{"line_number":279,"context_line":"                \u0027chunked\u0027):"},{"line_number":280,"context_line":"            request.is_body_readable \u003d True"},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a1ced50_c5d45fed","line":280,"range":{"start_line":278,"start_character":7,"end_line":280,"end_character":43},"updated":"2017-03-22 22:25:33.000000000","message":"Hemanth, were we not to remove this from here as we have added the check for chunked encoding explicitly in wsgi.py?","commit_id":"149e610fd2c2ca1e8ed17a02c737139b04e2aa12"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"5bbfa62bc7eef3513ed732abf6de9d18afe7bcc8","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if self.is_valid_encoding(request) and self.is_valid_method(request):"},{"line_number":279,"context_line":"            request.is_body_readable \u003d True"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"}],"source_content_type":"text/x-python","patch_set":9,"id":"ffe62b97_a76fa8fa","line":279,"range":{"start_line":278,"start_character":7,"end_line":279,"end_character":43},"updated":"2017-03-27 23:41:18.000000000","message":"This is what Ian was doing originally. Talk about circles!","commit_id":"463b3843d5e6339565735fb72d61d1beda39b5fc"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d54bcdaa526eeb9b647545ddaa2ec22c24c0fdfb","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        except exception.InvalidContentType as e:"},{"line_number":276,"context_line":"            raise webob.exc.HTTPUnsupportedMediaType(explanation\u003de.msg)"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if self.is_valid_encoding(request) and self.is_valid_method(request):"},{"line_number":279,"context_line":"            request.is_body_readable \u003d True"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        image_size \u003d request.content_length or None"},{"line_number":282,"context_line":"        return {\u0027size\u0027: image_size, \u0027data\u0027: request.body_file}"}],"source_content_type":"text/x-python","patch_set":9,"id":"ffe62b97_4757bc27","line":279,"range":{"start_line":278,"start_character":7,"end_line":279,"end_character":43},"in_reply_to":"ffe62b97_a76fa8fa","updated":"2017-03-27 23:42:56.000000000","message":"Around the world in 9 patch sets. :D","commit_id":"463b3843d5e6339565735fb72d61d1beda39b5fc"}],"glance/common/wsgi.py":[{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"1403e323adcc70f8df92b95b4d340729d99cede8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9a30ddce_4f878ab4","updated":"2017-03-13 16:38:34.000000000","message":"LGTM","commit_id":"e96145c37134d43af01e8ac23fc79ad47a5419e8"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"3d851d41a282a9f8c14ae7b42e201d2cf4878cbe","unresolved":false,"context_lines":[{"line_number":984,"context_line":"                                         \u0027gzip\u0027, \u0027identity\u0027])"},{"line_number":985,"context_line":"    httpverb_may_have_body \u003d frozenset({\u0027POST\u0027, \u0027PUT\u0027, \u0027PATCH\u0027})"},{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    def _handle_chunked_transfer_encoding(self, request):"},{"line_number":988,"context_line":"        request_encoding \u003d request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower()"},{"line_number":989,"context_line":"        is_valid_encoding \u003d request_encoding in self.valid_transfer_encoding"},{"line_number":990,"context_line":"        is_valid_method \u003d request.method.upper() in self.httpverb_may_have_body"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffe62b97_934af952","line":987,"updated":"2017-03-24 21:51:37.000000000","message":"may be a nit, but this method doesn\u0027t actually handle transfer-encoding, it evaluates the transfer encoding (many valid options, one is chunked). recommending a rename.","commit_id":"09e7cb4d0b86aac50c364075ac0569fe7a8d8bf4"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"72f5785527b3b38cfd95f863cafa121f6c1d7d42","unresolved":false,"context_lines":[{"line_number":984,"context_line":"                                         \u0027gzip\u0027, \u0027identity\u0027])"},{"line_number":985,"context_line":"    httpverb_may_have_body \u003d frozenset({\u0027POST\u0027, \u0027PUT\u0027, \u0027PATCH\u0027})"},{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    def _handle_chunked_transfer_encoding(self, request):"},{"line_number":988,"context_line":"        request_encoding \u003d request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower()"},{"line_number":989,"context_line":"        is_valid_encoding \u003d request_encoding in self.valid_transfer_encoding"},{"line_number":990,"context_line":"        is_valid_method \u003d request.method.upper() in self.httpverb_may_have_body"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffe62b97_b6472b1e","line":987,"in_reply_to":"ffe62b97_934af952","updated":"2017-03-24 22:16:28.000000000","message":"Sure","commit_id":"09e7cb4d0b86aac50c364075ac0569fe7a8d8bf4"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"3d851d41a282a9f8c14ae7b42e201d2cf4878cbe","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"        \"\"\""},{"line_number":1001,"context_line":"        self._handle_chunked_transfer_encoding(request)"},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"        request_encoding \u003d request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower()"},{"line_number":1004,"context_line":"        is_valid_encoding \u003d request_encoding in self.valid_transfer_encoding"},{"line_number":1005,"context_line":"        is_valid_method \u003d request.method.upper() in self.httpverb_may_have_body"},{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"        if is_valid_encoding and (request.is_body_readable or is_valid_method):"},{"line_number":1008,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffe62b97_b394dd8c","line":1005,"range":{"start_line":1003,"start_character":0,"end_line":1005,"end_character":79},"updated":"2017-03-24 21:51:37.000000000","message":"all of this duplication with the method above has a smell.\nwould prefer new (static) methods such as:\n\n  is_valid_encoding(request):\n\n  is_valid_method(request):\n\nthen relying on these in the implementation of the method above.\n\nyou are not even using is_valid_method here anymore (which is good, I was going to ask for this expression to be simplified.","commit_id":"09e7cb4d0b86aac50c364075ac0569fe7a8d8bf4"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"72f5785527b3b38cfd95f863cafa121f6c1d7d42","unresolved":false,"context_lines":[{"line_number":1000,"context_line":"        \"\"\""},{"line_number":1001,"context_line":"        self._handle_chunked_transfer_encoding(request)"},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"        request_encoding \u003d request.headers.get(\u0027transfer-encoding\u0027, \u0027\u0027).lower()"},{"line_number":1004,"context_line":"        is_valid_encoding \u003d request_encoding in self.valid_transfer_encoding"},{"line_number":1005,"context_line":"        is_valid_method \u003d request.method.upper() in self.httpverb_may_have_body"},{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"        if is_valid_encoding and (request.is_body_readable or is_valid_method):"},{"line_number":1008,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffe62b97_36655bb6","line":1005,"range":{"start_line":1003,"start_character":0,"end_line":1005,"end_character":79},"in_reply_to":"ffe62b97_b394dd8c","updated":"2017-03-24 22:16:28.000000000","message":"+1 working on that.","commit_id":"09e7cb4d0b86aac50c364075ac0569fe7a8d8bf4"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"369e68aaf88f59f55488c05688b247ad7c2abc67","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._evaluate_transfer_encoding(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffe62b97_4c84ce5e","line":1008,"range":{"start_line":1008,"start_character":8,"end_line":1008,"end_character":72},"updated":"2017-03-24 23:44:52.000000000","message":"Wont request.is_body_readable be set to True only if is_valid_method and is_valid_encoding both returned True as per L998.\n\nThen why call self.is_valid_encoding again here?\n\nWont this be enough?\n  if request.is_body_readable:","commit_id":"04bdc7891654e21341492bd616072d6a54a772f6"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"1c8937814dd0ff3b4008187117f8f07acc1b9cc9","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._evaluate_transfer_encoding(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffe62b97_ce2e8b42","line":1008,"range":{"start_line":1008,"start_character":8,"end_line":1008,"end_character":72},"in_reply_to":"ffe62b97_4c84ce5e","updated":"2017-03-25 13:02:46.000000000","message":"That requires us to set request.is_body_readable to False in _evaluate_transfer_encoding when method and/or encoding are/is not valid. We are not doing that currently and I\u0027m not sure if it has the same consequences. Needs more digging if we want to do that.","commit_id":"04bdc7891654e21341492bd616072d6a54a772f6"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"2750f100a450c4e65d0ac79148f929309e649209","unresolved":false,"context_lines":[{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"        :param request:  Webob.Request object"},{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._evaluate_transfer_encoding(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ffe62b97_e637f6fe","line":1009,"range":{"start_line":1006,"start_character":8,"end_line":1009,"end_character":23},"updated":"2017-03-27 21:42:47.000000000","message":"Something does not seem right here to me about the second explicit call to is_valid_encoding in L1008 after the overriding of the flag by us in L998.\nI feel we should not be overriding a flag value and then calling the same method that with an \"AND\" operator overrode it.","commit_id":"04bdc7891654e21341492bd616072d6a54a772f6"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"89c131e6e632a272cec61cab2b652a1a5e5443e2","unresolved":false,"context_lines":[{"line_number":995,"context_line":""},{"line_number":996,"context_line":"    def _decide_request_body_readable(self, request):"},{"line_number":997,"context_line":"        if self.is_valid_encoding(request) and self.is_valid_method(request):"},{"line_number":998,"context_line":"            request.is_body_readable \u003d True"},{"line_number":999,"context_line":""},{"line_number":1000,"context_line":"    def has_body(self, request):"},{"line_number":1001,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_e62016af","line":998,"range":{"start_line":998,"start_character":12,"end_line":998,"end_character":43},"updated":"2017-03-27 21:42:55.000000000","message":"return True","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"2344c24f81408e9615bc5083a4f00e6192c661a9","unresolved":false,"context_lines":[{"line_number":995,"context_line":""},{"line_number":996,"context_line":"    def _decide_request_body_readable(self, request):"},{"line_number":997,"context_line":"        if self.is_valid_encoding(request) and self.is_valid_method(request):"},{"line_number":998,"context_line":"            request.is_body_readable \u003d True"},{"line_number":999,"context_line":""},{"line_number":1000,"context_line":"    def has_body(self, request):"},{"line_number":1001,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_acce6fb1","line":998,"range":{"start_line":998,"start_character":12,"end_line":998,"end_character":43},"in_reply_to":"ffe62b97_4c1a436f","updated":"2017-03-27 22:24:26.000000000","message":"Yes. Hemanth\u0027s suggestion should work good.\nWe can avoid the explicit \"return False\" though.","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"6803fa2f88af98f40f233577021443707e196a43","unresolved":false,"context_lines":[{"line_number":995,"context_line":""},{"line_number":996,"context_line":"    def _decide_request_body_readable(self, request):"},{"line_number":997,"context_line":"        if self.is_valid_encoding(request) and self.is_valid_method(request):"},{"line_number":998,"context_line":"            request.is_body_readable \u003d True"},{"line_number":999,"context_line":""},{"line_number":1000,"context_line":"    def has_body(self, request):"},{"line_number":1001,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_4c1a436f","line":998,"range":{"start_line":998,"start_character":12,"end_line":998,"end_character":43},"in_reply_to":"ffe62b97_6193d05c","updated":"2017-03-27 22:11:32.000000000","message":"FWIW Hemanth\u0027s suggestion matches what I was imagining Dharini was intending: An addition of return true rather than replacement of the request.is_body_readable \u003d True\n\nthe addition of a ``return False`` follows implicitly","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"1bc0eb1d2103d6d101f77e0f0ed87dd25d3b1348","unresolved":false,"context_lines":[{"line_number":995,"context_line":""},{"line_number":996,"context_line":"    def _decide_request_body_readable(self, request):"},{"line_number":997,"context_line":"        if self.is_valid_encoding(request) and self.is_valid_method(request):"},{"line_number":998,"context_line":"            request.is_body_readable \u003d True"},{"line_number":999,"context_line":""},{"line_number":1000,"context_line":"    def has_body(self, request):"},{"line_number":1001,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_6193d05c","line":998,"range":{"start_line":998,"start_character":12,"end_line":998,"end_character":43},"in_reply_to":"ffe62b97_e62016af","updated":"2017-03-27 22:03:00.000000000","message":"-1\nThis doesn\u0027t solve our issue.\nAs I mentioned in the commit message, request.is_body_readable MUST be set in order to read the body.\n\nWhat you said would work if request.is_body_readable was being consumed only by Glance. It is specifically a signal to WebOb to treat bodies of requests with chunked encoding as readable. If we don\u0027t set this, WebOb 1.7 will always return an empty bytearray as the request body.\n\nHow about this:\n\n def _decide_request_body_readable(self, request):\n     if self.is_valid_encoding(request) and self.is_valid_method(request):\n         request.is_body_readable \u003d True\n         return True\n     else:\n         return False","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"b5e7712520c687a415296e76300c7f5d65b8c0c0","unresolved":false,"context_lines":[{"line_number":995,"context_line":""},{"line_number":996,"context_line":"    def _decide_request_body_readable(self, request):"},{"line_number":997,"context_line":"        if self.is_valid_encoding(request) and self.is_valid_method(request):"},{"line_number":998,"context_line":"            request.is_body_readable \u003d True"},{"line_number":999,"context_line":""},{"line_number":1000,"context_line":"    def has_body(self, request):"},{"line_number":1001,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_41bd6c00","line":998,"range":{"start_line":998,"start_character":12,"end_line":998,"end_character":43},"in_reply_to":"ffe62b97_e62016af","updated":"2017-03-27 21:51:48.000000000","message":"simplification: if this method is given a bool return value, it can just become \n\n  _is_body_readable(self, request):","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"89c131e6e632a272cec61cab2b652a1a5e5443e2","unresolved":false,"context_lines":[{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"        :param request:  Webob.Request object"},{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._decide_request_body_readable(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_46058213","line":1009,"range":{"start_line":1006,"start_character":8,"end_line":1009,"end_character":23},"updated":"2017-03-27 21:42:55.000000000","message":"if self._decide_request_body_readable(request) and request.is_body_readable:\n        return True","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"1927216943b6ea96905f8196bd2d70a9e4d122a0","unresolved":false,"context_lines":[{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"        :param request:  Webob.Request object"},{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._decide_request_body_readable(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_2cb37f4f","line":1009,"range":{"start_line":1006,"start_character":8,"end_line":1009,"end_character":23},"in_reply_to":"ffe62b97_214a68ab","updated":"2017-03-27 22:14:00.000000000","message":"sorry, meant to say that has_body becomes something like: \n\n  return request.is_body_readable \u003d self.is_valid_encoding(request) and self.is_valid_method(request)","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"3dfa25846259b8ec75e61f3143b99a2ea22548fb","unresolved":false,"context_lines":[{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"        :param request:  Webob.Request object"},{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._decide_request_body_readable(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_c7598c8e","line":1009,"range":{"start_line":1006,"start_character":8,"end_line":1009,"end_character":23},"in_reply_to":"ffe62b97_2c16dff2","updated":"2017-03-27 23:33:14.000000000","message":"I\u0027m having second thoughts here. I think what I suggested is a tad implicit. While it looks slick, I\u0027m thinking it\u0027s better to be explicit. I\u0027ll upload both changes as different patches. We can take a call after comparing. At this point, I\u0027m leaning towards explicit but ugly.","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"2344c24f81408e9615bc5083a4f00e6192c661a9","unresolved":false,"context_lines":[{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"        :param request:  Webob.Request object"},{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._decide_request_body_readable(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_2c16dff2","line":1009,"range":{"start_line":1006,"start_character":8,"end_line":1009,"end_character":23},"in_reply_to":"ffe62b97_2cb37f4f","updated":"2017-03-27 22:24:26.000000000","message":"Steve, as per your suggestion, we would be able to directly call our class methods but we will also have to set request.is_body_readable to True explicitly later on. Instead, I think it is cool to do:\n\nif self._decide_request_body_readable(request):\n  return True\n\njust as Hemanth suggested and have our _decide_request_body_readable, call our class methods and set the is_body_readable flag.","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"1bc0eb1d2103d6d101f77e0f0ed87dd25d3b1348","unresolved":false,"context_lines":[{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"        :param request:  Webob.Request object"},{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._decide_request_body_readable(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_214a68ab","line":1009,"range":{"start_line":1006,"start_character":8,"end_line":1009,"end_character":23},"in_reply_to":"ffe62b97_46058213","updated":"2017-03-27 22:03:00.000000000","message":"And, change this to:\n\n if self._decide_request_body_readable(request):\n     return True","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"b5e7712520c687a415296e76300c7f5d65b8c0c0","unresolved":false,"context_lines":[{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"        :param request:  Webob.Request object"},{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        self._decide_request_body_readable(request)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"        if self.is_valid_encoding(request) and request.is_body_readable:"},{"line_number":1009,"context_line":"            return True"},{"line_number":1010,"context_line":"        elif request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1011,"context_line":"            return True"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_01be44d4","line":1009,"range":{"start_line":1006,"start_character":8,"end_line":1009,"end_character":23},"in_reply_to":"ffe62b97_46058213","updated":"2017-03-27 21:51:48.000000000","message":"regarding Dharini\u0027s suggestions, the second condition doesn\u0027t seem to make sense to test at all anymore. \n\nAt that point it seems like we could just inline the logic of _is_body_readable(self, request) and get rid of that method. has_body will just become:\n\n  return self.is_valid_encoding(request) and self.is_valid_method(request)\n\nAre we really sure this is the correct logic?","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"83ec8994e79c99b9569c5597ca37088e7eb4ab65","unresolved":false,"context_lines":[{"line_number":998,"context_line":"        if cls.is_valid_encoding(request) and cls.is_valid_method(request):"},{"line_number":999,"context_line":"            request.is_body_readable \u003d True"},{"line_number":1000,"context_line":"            return True"},{"line_number":1001,"context_line":"        else:"},{"line_number":1002,"context_line":"            return False"},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    def has_body(self, request):"},{"line_number":1005,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"ffe62b97_596b2484","line":1002,"range":{"start_line":1001,"start_character":8,"end_line":1002,"end_character":24},"updated":"2017-03-28 00:21:31.000000000","message":"This is not necessary but it is okay to be explicit.","commit_id":"0bd7e98f3a5417f0444c8ba81f75b441c0b44463"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"83ec8994e79c99b9569c5597ca37088e7eb4ab65","unresolved":false,"context_lines":[{"line_number":1011,"context_line":"        if self._decide_request_body_readable(request):"},{"line_number":1012,"context_line":"            return True"},{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        if request.content_length is not None and request.content_length \u003e 0:"},{"line_number":1015,"context_line":"            return True"},{"line_number":1016,"context_line":""},{"line_number":1017,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffe62b97_f9a798bc","line":1014,"range":{"start_line":1014,"start_character":8,"end_line":1014,"end_character":10},"updated":"2017-03-28 00:21:31.000000000","message":"Nice catch to make it if-if and have an outer return as in L1017.\n\nAlternatively, if-elif-else.","commit_id":"0bd7e98f3a5417f0444c8ba81f75b441c0b44463"}],"glance/tests/unit/common/test_wsgi.py":[{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"89c131e6e632a272cec61cab2b652a1a5e5443e2","unresolved":false,"context_lines":[{"line_number":517,"context_line":"    def test_has_body_valid_transfer_encoding_without_content_length(self):"},{"line_number":518,"context_line":"        self.assertTrue(self._check_transfer_encoding("},{"line_number":519,"context_line":"                        transfer_encoding\u003d\u0027chunked\u0027))"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"    def _check_transfer_encoding(self, transfer_encoding\u003dNone,"},{"line_number":522,"context_line":"                                 content_length\u003dNone):"},{"line_number":523,"context_line":"        request \u003d wsgi.Request.blank(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_4148cc5a","line":520,"updated":"2017-03-27 21:42:55.000000000","message":"Can we add a test \"test_has_body_invalid_transfer_encoding_without_content_length\" and assert that it remains False?","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"1bc0eb1d2103d6d101f77e0f0ed87dd25d3b1348","unresolved":false,"context_lines":[{"line_number":517,"context_line":"    def test_has_body_valid_transfer_encoding_without_content_length(self):"},{"line_number":518,"context_line":"        self.assertTrue(self._check_transfer_encoding("},{"line_number":519,"context_line":"                        transfer_encoding\u003d\u0027chunked\u0027))"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"    def _check_transfer_encoding(self, transfer_encoding\u003dNone,"},{"line_number":522,"context_line":"                                 content_length\u003dNone):"},{"line_number":523,"context_line":"        request \u003d wsgi.Request.blank(\u0027/\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ffe62b97_8175b466","line":520,"in_reply_to":"ffe62b97_4148cc5a","updated":"2017-03-27 22:03:00.000000000","message":"+1\nGood suggestion","commit_id":"d3cf831c23d7b90ddf80ab172b90df90007d26cf"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"83ec8994e79c99b9569c5597ca37088e7eb4ab65","unresolved":false,"context_lines":[{"line_number":508,"context_line":""},{"line_number":509,"context_line":"    def test_has_body_invalid_transfer_encoding_no_content_len_and_body(self):"},{"line_number":510,"context_line":"        self.assertFalse(self._check_transfer_encoding("},{"line_number":511,"context_line":"                        transfer_encoding\u003d\u0027invalid\u0027, include_body\u003dFalse))"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"    def test_has_body_invalid_transfer_encoding_no_content_len_but_body(self):"},{"line_number":514,"context_line":"        self.assertTrue(self._check_transfer_encoding("}],"source_content_type":"text/x-python","patch_set":8,"id":"ffe62b97_99752c67","line":511,"range":{"start_line":511,"start_character":24,"end_line":511,"end_character":25},"updated":"2017-03-28 00:21:31.000000000","message":"indent 1 space.","commit_id":"0bd7e98f3a5417f0444c8ba81f75b441c0b44463"}]}