)]}'
{"glance/api/v1/images.py":[{"author":{"_account_id":11428,"name":"Ajaya Agrawal","email":"ajku.agr@gmail.com","username":"ajayaa"},"change_message_id":"9cf6e4af59cc7183cdd327b2f6c46b6aa5066ee6","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":93,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":94,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":95,"context_line":"               checksum)"},{"line_number":96,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    amazon_formats \u003d (\u0027aki\u0027, \u0027ari\u0027, \u0027ami\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_de5097d6","line":95,"updated":"2015-05-19 21:31:39.000000000","message":"I see that we are not logging these validation failures. Should not we do that?","commit_id":"b75609c80a36e6725244f915df8f2c959fc864ec"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"6434b9cc47b421baaa2d9835388978db47f0d47d","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":93,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":94,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":95,"context_line":"               checksum)"},{"line_number":96,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    amazon_formats \u003d (\u0027aki\u0027, \u0027ari\u0027, \u0027ami\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_617ae289","line":95,"in_reply_to":"7a016987_de5097d6","updated":"2015-05-22 12:40:00.000000000","message":"I think it will be the right thing to do. What do you think, is \u0027warning\u0027 level the right loglevel?","commit_id":"b75609c80a36e6725244f915df8f2c959fc864ec"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"863c26d67f2f327e4014c1ea8302a2611d390b18","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    if \u0027disk_format\u0027 in values:"},{"line_number":76,"context_line":"        if disk_format not in CONF.image_format.disk_formats:"},{"line_number":77,"context_line":"            msg \u003d _(\"Invalid disk format \u0027%s\u0027 for image.\") % disk_format"},{"line_number":78,"context_line":"            LOG.warn(msg)"},{"line_number":79,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    if \u0027container_format\u0027 in values:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_359aa515","line":78,"updated":"2015-06-02 07:09:41.000000000","message":"ditto","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"7ada3e255181c4798a29f844316dc885a7858135","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    if \u0027disk_format\u0027 in values:"},{"line_number":76,"context_line":"        if disk_format not in CONF.image_format.disk_formats:"},{"line_number":77,"context_line":"            msg \u003d _(\"Invalid disk format \u0027%s\u0027 for image.\") % disk_format"},{"line_number":78,"context_line":"            LOG.warn(msg)"},{"line_number":79,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    if \u0027container_format\u0027 in values:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3afb71cf_9d5e13e8","line":78,"in_reply_to":"5afe65bd_359aa515","updated":"2015-06-05 16:28:04.000000000","message":"Done","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"863c26d67f2f327e4014c1ea8302a2611d390b18","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        if container_format not in CONF.image_format.container_formats:"},{"line_number":83,"context_line":"            msg \u003d _(\"Invalid container format \u0027%s\u0027 \""},{"line_number":84,"context_line":"                    \"for image.\") % container_format"},{"line_number":85,"context_line":"            LOG.warn(msg)"},{"line_number":86,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    if name and len(name) \u003e 255:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_559521e2","line":85,"updated":"2015-06-02 07:09:41.000000000","message":"ditto","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"7ada3e255181c4798a29f844316dc885a7858135","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        if container_format not in CONF.image_format.container_formats:"},{"line_number":83,"context_line":"            msg \u003d _(\"Invalid container format \u0027%s\u0027 \""},{"line_number":84,"context_line":"                    \"for image.\") % container_format"},{"line_number":85,"context_line":"            LOG.warn(msg)"},{"line_number":86,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    if name and len(name) \u003e 255:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3afb71cf_7d6357b2","line":85,"in_reply_to":"5afe65bd_559521e2","updated":"2015-06-05 16:28:04.000000000","message":"Done","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"863c26d67f2f327e4014c1ea8302a2611d390b18","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":97,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":98,"context_line":"               checksum)"},{"line_number":99,"context_line":"        LOG.warn(msg)"},{"line_number":100,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    amazon_formats \u003d (\u0027aki\u0027, \u0027ari\u0027, \u0027ami\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_d502313e","line":99,"updated":"2015-06-02 07:09:41.000000000","message":"Is there need to log this message? Any bad user then can use this to flood the log files by calling this N number of times.","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"7ada3e255181c4798a29f844316dc885a7858135","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":97,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":98,"context_line":"               checksum)"},{"line_number":99,"context_line":"        LOG.warn(msg)"},{"line_number":100,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    amazon_formats \u003d (\u0027aki\u0027, \u0027ari\u0027, \u0027ami\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3afb71cf_5d589bfa","line":99,"in_reply_to":"5afe65bd_d502313e","updated":"2015-06-05 16:28:04.000000000","message":"Sounds reasonable.","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"863c26d67f2f327e4014c1ea8302a2611d390b18","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                     \"When setting a disk or container format to \""},{"line_number":112,"context_line":"                     \"one of \u0027aki\u0027, \u0027ari\u0027, or \u0027ami\u0027, the container \""},{"line_number":113,"context_line":"                     \"and disk formats must match.\"))"},{"line_number":114,"context_line":"            LOG.warn(msg)"},{"line_number":115,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    return values"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_75f93d4d","line":114,"updated":"2015-06-02 07:09:41.000000000","message":"ditto","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"7ada3e255181c4798a29f844316dc885a7858135","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                     \"When setting a disk or container format to \""},{"line_number":112,"context_line":"                     \"one of \u0027aki\u0027, \u0027ari\u0027, or \u0027ami\u0027, the container \""},{"line_number":113,"context_line":"                     \"and disk formats must match.\"))"},{"line_number":114,"context_line":"            LOG.warn(msg)"},{"line_number":115,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    return values"}],"source_content_type":"text/x-python","patch_set":2,"id":"3afb71cf_3d5ddfe9","line":114,"in_reply_to":"5afe65bd_75f93d4d","updated":"2015-06-05 16:28:04.000000000","message":"Done","commit_id":"6930df8e0042cab72b38b895c648e27223a325f7"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"08387340775a45b6b5306195d9763482cfdfcc75","unresolved":false,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def _check_int_param(param):"},{"line_number":122,"context_line":"        # Integer field can store max 8-byte signed integer"},{"line_number":123,"context_line":"        if param and int(param) \u003e pow(2, 63) - 1:"},{"line_number":124,"context_line":"            msg \u003d _(\"\u0027%s\u0027 value out of range: \""},{"line_number":125,"context_line":"                    \"expected 8-bytes signed integer\") % param"},{"line_number":126,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3afb71cf_0913e667","line":123,"updated":"2015-06-08 06:17:38.000000000","message":"Hi,\n\nI have tested this patch with pow(2,63) -1 and it is still raising 500 error. It should be pow(2,31) -1.\n\nAlso, IMO this validation should be done in db api layer, please refer comment https://review.openstack.org/#/c/165068/4/cinder/api/contrib/quotas.py\n\nWhat do you think?","commit_id":"f83a8e98c3d4ce35c80758c29393405bdaabfa5c"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"f2892e06d8583b8acc04b156a3026d5f046b075f","unresolved":false,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def _check_int_param(param):"},{"line_number":122,"context_line":"        # Integer field can store max 8-byte signed integer"},{"line_number":123,"context_line":"        if param and int(param) \u003e pow(2, 63) - 1:"},{"line_number":124,"context_line":"            msg \u003d _(\"\u0027%s\u0027 value out of range: \""},{"line_number":125,"context_line":"                    \"expected 8-bytes signed integer\") % param"},{"line_number":126,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3afb71cf_5043a842","line":123,"in_reply_to":"3afb71cf_0913e667","updated":"2015-06-08 15:41:01.000000000","message":"+1 for doing the validation in db api layer. Actually, I had the same idea from the very beginning, but I chose the wrong way to do everything in one place in the api along with other params validation.\n\nAs for the fact that it should be pow(2, 31) - 1: I consulted the sqlite docs (http://www.sqlite.org/datatype3.html) and INTEGER field there can store values up to 8 bytes long. But, alas, mysql docs (http://dev.mysql.com/doc/refman/5.5/en/integer-types.html) that INT is stored in 4 bytes.\n\nSo I guess you are right here as well and my idea to resort to checking sys.maxint failed. I\u0027ll produce the fix in the next PS.","commit_id":"f83a8e98c3d4ce35c80758c29393405bdaabfa5c"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"63c8a3f94ecbae3ab5ba6dc8928826015bca3024","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # check that checksum retrieved is exactly 32 characters"},{"line_number":112,"context_line":"    # as long as we expect md5 checksum"},{"line_number":113,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":114,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":115,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":116,"context_line":"               checksum)"},{"line_number":117,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_eab73fd6","line":114,"updated":"2015-06-08 17:51:52.000000000","message":"I would recommend to use regexp here to verify that checksum is md5 hash, rather than just checking length of the string. Kind of:\n\nre.findall(r\"([a-fA-F\\d]{32})\", checksum)","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"8764b19e5de808a1c5f6cb84c33ab71e8a2ddc9a","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # check that checksum retrieved is exactly 32 characters"},{"line_number":112,"context_line":"    # as long as we expect md5 checksum"},{"line_number":113,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":114,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":115,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":116,"context_line":"               checksum)"},{"line_number":117,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_124df7c3","line":114,"in_reply_to":"3afb71cf_36f7f2f6","updated":"2015-06-09 17:31:02.000000000","message":"Sorry to point out, but that\u0027s your code, Mike :)","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":8127,"name":"Alexander Tivelkov","email":"locher@yandex.ru","username":"ativelkov"},"change_message_id":"b76007039ed5b4ed2d36cc7c009a6226838184b4","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # check that checksum retrieved is exactly 32 characters"},{"line_number":112,"context_line":"    # as long as we expect md5 checksum"},{"line_number":113,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":114,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":115,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":116,"context_line":"               checksum)"},{"line_number":117,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_fabf0af9","line":114,"in_reply_to":"3afb71cf_eab73fd6","updated":"2015-06-08 18:01:04.000000000","message":"I don\u0027t agree.\nThis will implicitly introduce an algorithm dependency here, which assumes particular MD5 hashing algorithm, but since this is a user-assignable field, there may be a theoretical possibility to have some other string as a \"hash-sum\", not necessarily the hex number.\nWhat we should do here is to ensure that the user-provided values will not cause DBError dues to \"value too long\" exception.\n\nI would even replace !\u003d32 with \u003e32, to allow some shorter strings to be passed as \"checksums\".  But this does not worth a -1 here, I believe.","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"ac2f40d03bed367f422af6fd1bf69034a47ea36d","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # check that checksum retrieved is exactly 32 characters"},{"line_number":112,"context_line":"    # as long as we expect md5 checksum"},{"line_number":113,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":114,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":115,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":116,"context_line":"               checksum)"},{"line_number":117,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_fa56ead0","line":114,"in_reply_to":"3afb71cf_eab73fd6","updated":"2015-06-08 18:06:45.000000000","message":"Let me disagree: a regexp at this stage is an overkill as we should not invent our-own-invented-here md5checksum checker. And unless we do so, what\u0027s the point of checking that md5 checksum only looks like one?","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"9c7fcb95cd4ae4d7156aa1b9f6f089a90ec76cb7","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # check that checksum retrieved is exactly 32 characters"},{"line_number":112,"context_line":"    # as long as we expect md5 checksum"},{"line_number":113,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":114,"context_line":"    if checksum and len(checksum) !\u003d 32:"},{"line_number":115,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: expected 32-characters long md5\") %"},{"line_number":116,"context_line":"               checksum)"},{"line_number":117,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_36f7f2f6","line":114,"in_reply_to":"3afb71cf_fa56ead0","updated":"2015-06-08 22:23:19.000000000","message":"So if we are afraid of regexp in very rare cases, why do we use it permanently in artifacts on every request? https://review.openstack.org/#/c/136629/156/glance/api/v3/router.py\n\nI see double standards here :p","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":16683,"name":"Alexey Galkin","email":"agalkin@mirantis.com","username":"agalkin"},"change_message_id":"b863a27c342329cf43372957af58ea36367bfc11","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # check that checksum retrieved is exactly 32 characters"},{"line_number":112,"context_line":"    # as long as we expect md5 checksum"},{"line_number":113,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":114,"context_line":"    if checksum and len(checksum) \u003e 32:"},{"line_number":115,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: can\u0027t exceed 32 characters\") %"},{"line_number":116,"context_line":"               checksum)"},{"line_number":117,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3afb71cf_dccaaa1a","line":114,"updated":"2015-06-10 13:43:19.000000000","message":"I think you need to consider another way to check the validity of the checksum. Something like this:\nif re.match(r\"^([a-fA-F\\d]{32})$\", checksum):","commit_id":"551ecf206c77e1bb940d7517a0681de2b3c8e65d"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"37503abc20fdffadbcff91fc1c4c5f383caf3136","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    # check that checksum retrieved is exactly 32 characters"},{"line_number":112,"context_line":"    # as long as we expect md5 checksum"},{"line_number":113,"context_line":"    # https://bugs.launchpad.net/glance/+bug/1454730"},{"line_number":114,"context_line":"    if checksum and len(checksum) \u003e 32:"},{"line_number":115,"context_line":"        msg \u003d (_(\"Invalid checksum \u0027%s\u0027: can\u0027t exceed 32 characters\") %"},{"line_number":116,"context_line":"               checksum)"},{"line_number":117,"context_line":"        raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3afb71cf_97d3ebd4","line":114,"in_reply_to":"3afb71cf_dccaaa1a","updated":"2015-06-10 14:02:13.000000000","message":"Alex, this has been already discussed - https://review.openstack.org/#/c/182703/9/glance/api/v1/images.py\n\nThere is no need for checksum validation here, we only need to check that the length of the supplied string doesn\u0027t violate the DB constraint.","commit_id":"551ecf206c77e1bb940d7517a0681de2b3c8e65d"}],"glance/db/sqlalchemy/api.py":[{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"26f04599a02672140366fcbed165617510f6c5a6","unresolved":false,"context_lines":[{"line_number":680,"context_line":"        param \u003d values.get(param_str)"},{"line_number":681,"context_line":"        # the maximum value of Integer field is pow(2, 31) - 1 as mysql INT is"},{"line_number":682,"context_line":"        # stored in 4 bytes"},{"line_number":683,"context_line":"        max_int \u003d pow(2, 31) - 1"},{"line_number":684,"context_line":"        if param and param \u003e max_int:"},{"line_number":685,"context_line":"            msg \u003d _(\"\u0027%(param)s\u0027 value out of range, \""},{"line_number":686,"context_line":"                    \"must not exceed %(value)d\") % {\"param\": param_str,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_f6160cf0","line":683,"updated":"2015-06-08 15:49:18.000000000","message":"This should be calculated once, outside of the loop. Also there\u0027s no need to use the pow() function. The code should look like:\n\n    # Your comments here\n    max_int \u003d (2**31) -1  # If you want to be explicit. ** has higher precedence though\n    # your comments here\n    for param_str in (\u0027min_disk\u0027, \u0027min_ram\u0027):\n        param \u003d values.get(param_str)\n        if param and param \u003e max_int:\n            # ... etc.","commit_id":"abe4ce315a92cff02a3076ee3f5aaf441fb4b93e"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"5717a412157e622bf26dd16a72d3fb79200a977c","unresolved":false,"context_lines":[{"line_number":680,"context_line":"        param \u003d values.get(param_str)"},{"line_number":681,"context_line":"        # the maximum value of Integer field is pow(2, 31) - 1 as mysql INT is"},{"line_number":682,"context_line":"        # stored in 4 bytes"},{"line_number":683,"context_line":"        max_int \u003d pow(2, 31) - 1"},{"line_number":684,"context_line":"        if param and param \u003e max_int:"},{"line_number":685,"context_line":"            msg \u003d _(\"\u0027%(param)s\u0027 value out of range, \""},{"line_number":686,"context_line":"                    \"must not exceed %(value)d\") % {\"param\": param_str,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3afb71cf_b693f44e","line":683,"in_reply_to":"3afb71cf_f6160cf0","updated":"2015-06-08 16:03:27.000000000","message":"Yeah, putting it inside the loop is not a good idea.","commit_id":"abe4ce315a92cff02a3076ee3f5aaf441fb4b93e"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"4cad180e5b5c40207b67dc157113b07d5c233ae1","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        msg \u003d \"Invalid image status \u0027%s\u0027 for image.\" % status"},{"line_number":676,"context_line":"        raise exception.Invalid(msg)"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"    # validate integer values to eliminate DBError on save"},{"line_number":679,"context_line":"    # the maximum value of Integer field is (2 ** 31) - 1 as mysql INT is"},{"line_number":680,"context_line":"    # stored in 4 bytes"},{"line_number":681,"context_line":"    max_int \u003d (2 ** 31) - 1"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_1c751031","line":678,"updated":"2015-06-09 16:41:09.000000000","message":"Can we please move this validation to a common method that other resource can use for sanity checks for eg. metadef namespace id?","commit_id":"878f1a9453fd7e2b5b8eb9a33cbbdfff6304c293"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"e21245d7182a593e07f8cae35f3cc825a490b372","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        msg \u003d \"Invalid image status \u0027%s\u0027 for image.\" % status"},{"line_number":676,"context_line":"        raise exception.Invalid(msg)"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"    # validate integer values to eliminate DBError on save"},{"line_number":679,"context_line":"    # the maximum value of Integer field is (2 ** 31) - 1 as mysql INT is"},{"line_number":680,"context_line":"    # stored in 4 bytes"},{"line_number":681,"context_line":"    max_int \u003d (2 ** 31) - 1"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_7edab5e9","line":678,"in_reply_to":"3afb71cf_1c751031","updated":"2015-06-09 18:36:32.000000000","message":"Done, moved to glance.common.utils.\n\nHope I found nice tradeoff between YAGNI principle and reusability :)","commit_id":"878f1a9453fd7e2b5b8eb9a33cbbdfff6304c293"}],"glance/tests/functional/db/base.py":[{"author":{"_account_id":16683,"name":"Alexey Galkin","email":"agalkin@mirantis.com","username":"agalkin"},"change_message_id":"2254079a2eb29437ae1b9026f42a5c775ffe0dd3","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        self.assertRaises(exception.Invalid, self.db_api.image_create,"},{"line_number":223,"context_line":"                          self.context, fixture)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    def test_image_create_bad_checksum(self):"},{"line_number":226,"context_line":"        # checksum should be no longer than 32 characters"},{"line_number":227,"context_line":"        bad_checksum \u003d \"42\" * 42"},{"line_number":228,"context_line":"        fixture \u003d {\u0027checksum\u0027: bad_checksum}"}],"source_content_type":"text/x-python","patch_set":11,"id":"3afb71cf_d731635b","line":225,"updated":"2015-06-10 14:00:54.000000000","message":"Сhecksum can be not only a large number of characters, but also extraneous/incorrect symbols","commit_id":"551ecf206c77e1bb940d7517a0681de2b3c8e65d"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"5c603c9d518a81c89fddd68400bb90b516a8c0ac","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        self.assertRaises(exception.Invalid, self.db_api.image_create,"},{"line_number":223,"context_line":"                          self.context, fixture)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"    def test_image_create_bad_checksum(self):"},{"line_number":226,"context_line":"        # checksum should be no longer than 32 characters"},{"line_number":227,"context_line":"        bad_checksum \u003d \"42\" * 42"},{"line_number":228,"context_line":"        fixture \u003d {\u0027checksum\u0027: bad_checksum}"}],"source_content_type":"text/x-python","patch_set":11,"id":"3afb71cf_e892404b","line":225,"in_reply_to":"3afb71cf_d731635b","updated":"2015-06-10 14:57:25.000000000","message":"Alex, imho the check for all possible extraneous symbols is out of the scope of this patch.","commit_id":"551ecf206c77e1bb940d7517a0681de2b3c8e65d"}],"glance/tests/functional/v1/test_api.py":[{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"40103442ea8692fc14bc0a05b507646ba189a463","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    \"\"\"Functional tests using httplib2 against the API server\"\"\""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def test_checksum_not_32_chars_at_image_create(self):"},{"line_number":39,"context_line":"        self.cleanup()"},{"line_number":40,"context_line":"        self.start_servers(**self.__dict__.copy())"},{"line_number":41,"context_line":"        headers \u003d minimal_headers(\u0027Image1\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a0c5dc1_427d2fe6","line":38,"updated":"2015-05-13 15:00:30.000000000","message":"how about adding a positive test case too?","commit_id":"b75609c80a36e6725244f915df8f2c959fc864ec"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"6434b9cc47b421baaa2d9835388978db47f0d47d","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    \"\"\"Functional tests using httplib2 against the API server\"\"\""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def test_checksum_not_32_chars_at_image_create(self):"},{"line_number":39,"context_line":"        self.cleanup()"},{"line_number":40,"context_line":"        self.start_servers(**self.__dict__.copy())"},{"line_number":41,"context_line":"        headers \u003d minimal_headers(\u0027Image1\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_016c4ecc","line":38,"in_reply_to":"9a0c5dc1_427d2fe6","updated":"2015-05-22 12:40:00.000000000","message":"I think it\u0027s a bit out of the scope of the bug as long as we are dealing with value too long failure, and not checksum validation issues. Though it will increase coverage, which is good. I\u0027ll do my best.","commit_id":"b75609c80a36e6725244f915df8f2c959fc864ec"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"63c8a3f94ecbae3ab5ba6dc8928826015bca3024","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        # Integer field can\u0027t be greater than max 8-byte signed integer"},{"line_number":75,"context_line":"        for param in [\u0027min_disk\u0027, \u0027min_ram\u0027]:"},{"line_number":76,"context_line":"            headers \u003d minimal_headers(\u0027Image1\u0027)"},{"line_number":77,"context_line":"            headers[\u0027X-Image-Meta-%s\u0027 % param] \u003d str(sys.maxint + 1)"},{"line_number":78,"context_line":"            content \u003d self._check_image_create(headers, 400)"},{"line_number":79,"context_line":"            self.assertIn(\"value out of range\", content)"},{"line_number":80,"context_line":"            # verify positive case as well"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_65df006a","line":77,"updated":"2015-06-08 17:51:52.000000000","message":"Why do you use sys.maxint here if below you strictly hardcode it to (2 ** 31) - 1 ?","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"ac2f40d03bed367f422af6fd1bf69034a47ea36d","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        # Integer field can\u0027t be greater than max 8-byte signed integer"},{"line_number":75,"context_line":"        for param in [\u0027min_disk\u0027, \u0027min_ram\u0027]:"},{"line_number":76,"context_line":"            headers \u003d minimal_headers(\u0027Image1\u0027)"},{"line_number":77,"context_line":"            headers[\u0027X-Image-Meta-%s\u0027 % param] \u003d str(sys.maxint + 1)"},{"line_number":78,"context_line":"            content \u003d self._check_image_create(headers, 400)"},{"line_number":79,"context_line":"            self.assertIn(\"value out of range\", content)"},{"line_number":80,"context_line":"            # verify positive case as well"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_7ae8bab4","line":77,"in_reply_to":"3afb71cf_65df006a","updated":"2015-06-08 18:06:45.000000000","message":"Well, I wanted to make sure that the test will likely fail. sys.maxint + 1 will be always greater than pow(2, 31) - 1. \n\nThough a case n in [pow(2, 31), sys.maxint] may also be checked.","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"9c7fcb95cd4ae4d7156aa1b9f6f089a90ec76cb7","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        # Integer field can\u0027t be greater than max 8-byte signed integer"},{"line_number":75,"context_line":"        for param in [\u0027min_disk\u0027, \u0027min_ram\u0027]:"},{"line_number":76,"context_line":"            headers \u003d minimal_headers(\u0027Image1\u0027)"},{"line_number":77,"context_line":"            headers[\u0027X-Image-Meta-%s\u0027 % param] \u003d str(sys.maxint + 1)"},{"line_number":78,"context_line":"            content \u003d self._check_image_create(headers, 400)"},{"line_number":79,"context_line":"            self.assertIn(\"value out of range\", content)"},{"line_number":80,"context_line":"            # verify positive case as well"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_b63082e9","line":77,"in_reply_to":"3afb71cf_7ae8bab4","updated":"2015-06-08 22:23:19.000000000","message":"pow(2, 31) will be always greater than pow(2, 31) - 1 too :) (math schoolbook told me so) what\u0027s the point then?","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"8764b19e5de808a1c5f6cb84c33ab71e8a2ddc9a","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        # Integer field can\u0027t be greater than max 8-byte signed integer"},{"line_number":75,"context_line":"        for param in [\u0027min_disk\u0027, \u0027min_ram\u0027]:"},{"line_number":76,"context_line":"            headers \u003d minimal_headers(\u0027Image1\u0027)"},{"line_number":77,"context_line":"            headers[\u0027X-Image-Meta-%s\u0027 % param] \u003d str(sys.maxint + 1)"},{"line_number":78,"context_line":"            content \u003d self._check_image_create(headers, 400)"},{"line_number":79,"context_line":"            self.assertIn(\"value out of range\", content)"},{"line_number":80,"context_line":"            # verify positive case as well"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_124857cb","line":77,"in_reply_to":"3afb71cf_b63082e9","updated":"2015-06-09 17:31:02.000000000","message":"My choice for sys.maxint + 1 is caused by the necessity of introducing a test for long value (2 ** 31 - 1 that you are proposing is not long). As for the value in [2 ** 31, sys.maxint] that you are proposing - I already covered this case in PS 10, and I\u0027ve already mentioned that.","commit_id":"20892e358895d0db75d02743d5748b1fc41f27b1"}]}