)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"57e000ce4e075c6e48614a637352c1c8b3fa5ad4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c2bb785c_3f7d8044","updated":"2022-05-23 03:33:10.000000000","message":"I think this is worth a respin for a couple of comments inline.  \n\nI\u0027m not really opposed to removing it, if someone can confirm the keys can still only be 255 chars long, that would be interesting and maybe we should keep that check here.","commit_id":"19ef1a096ff3f77069448f6bd23e874064814ddd"},{"author":{"_account_id":33934,"name":"Joshua Watt","email":"JPEWhacker@gmail.com","username":"jpew"},"change_message_id":"ab3af0b3191bd67bf330e0a42193f775200b7ce2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0f5f568b_ad6ae729","updated":"2022-05-20 18:19:45.000000000","message":"I tried this on our version of OpenStack and it allowed us to add more than 5 metadata properties.","commit_id":"19ef1a096ff3f77069448f6bd23e874064814ddd"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"a2de2bcc572137e2707f981c23c55b19a4731677","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"70f14f64_5a148f9d","updated":"2022-05-20 20:36:01.000000000","message":"Not approving as there is some hope ianw can review this and make sure we aren\u0027t missing something important. Also left a note that may want to be addressed.","commit_id":"19ef1a096ff3f77069448f6bd23e874064814ddd"}],"doc/source/openstack.rst":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"a2de2bcc572137e2707f981c23c55b19a4731677","unresolved":true,"context_lines":[{"line_number":258,"context_line":"        :type: dict"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        Arbitrary key/value metadata to store for this server using"},{"line_number":261,"context_line":"        the Nova metadata service."},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"     .. attr:: connection-type"},{"line_number":264,"context_line":"        :type: string"}],"source_content_type":"text/x-rst","patch_set":2,"id":"84e81241_34ca0ff2","line":261,"updated":"2022-05-20 20:36:01.000000000","message":"Nit, since we\u0027re updating this documentation we should update this to reflect it sets glance image properties not server instance metadata.\n\nSomething like:\n\n  Arbitrary key/value metadata to store as glance image properties.","commit_id":"19ef1a096ff3f77069448f6bd23e874064814ddd"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"57e000ce4e075c6e48614a637352c1c8b3fa5ad4","unresolved":true,"context_lines":[{"line_number":258,"context_line":"        :type: dict"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        Arbitrary key/value metadata to store for this server using"},{"line_number":261,"context_line":"        the Nova metadata service."},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"     .. attr:: connection-type"},{"line_number":264,"context_line":"        :type: string"}],"source_content_type":"text/x-rst","patch_set":2,"id":"4497b123_0104a2a9","line":261,"in_reply_to":"84e81241_34ca0ff2","updated":"2022-05-23 03:33:10.000000000","message":"++ I think this is worth fixing up here.  It is mighty confusing as is.","commit_id":"19ef1a096ff3f77069448f6bd23e874064814ddd"}],"nodepool/driver/openstack/config.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"57e000ce4e075c6e48614a637352c1c8b3fa5ad4","unresolved":true,"context_lines":[{"line_number":269,"context_line":"            # per Nova API rules"},{"line_number":270,"context_line":"            if i.meta:"},{"line_number":271,"context_line":"                if len(i.meta) \u003e 5 or \\"},{"line_number":272,"context_line":"                   any([len(k) \u003e 255 or len(v) \u003e 255"},{"line_number":273,"context_line":"                        for k, v in i.meta.items()]):"},{"line_number":274,"context_line":"                    # soft-fail"},{"line_number":275,"context_line":"                    # self.log.error(\"Invalid metadata for %s; ignored\""},{"line_number":276,"context_line":"                    #               % i.name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"54b45231_94b15d2f","side":"PARENT","line":273,"range":{"start_line":272,"start_character":19,"end_line":273,"end_character":53},"updated":"2022-05-23 03:33:10.000000000","message":"I think that this 255 characters is still (partly) the case?  It\u0027s all so long ago ...\n\nBut I think that nova used to enforce these limits, and over the years of API movement it has been passed of to glance directly.\n\nI think that the original check was removed with Ibec92e278887cd06e91687ca91e75f9b7b28098c which used to be in the create_image() path and enforced 255 for the property and value\n\nhttps://docs.openstack.org/api-ref/image/v2/index.html?expanded\u003dcreate-image-detail#request still says\n\n\"Additionally, you may include additional properties specified as key:value pairs, where the value must be a string data type. Keys are limited to 255 chars in length.\"\n\nSo maybe the value part is irrelevant, but the key length checking is still valid?","commit_id":"5551f99ca2aa08aba566f463d1ffa86344e61b35"},{"author":{"_account_id":33934,"name":"Joshua Watt","email":"JPEWhacker@gmail.com","username":"jpew"},"change_message_id":"a72c435cb0f0774117e1cc75ba58a4aab7e56d08","unresolved":true,"context_lines":[{"line_number":269,"context_line":"            # per Nova API rules"},{"line_number":270,"context_line":"            if i.meta:"},{"line_number":271,"context_line":"                if len(i.meta) \u003e 5 or \\"},{"line_number":272,"context_line":"                   any([len(k) \u003e 255 or len(v) \u003e 255"},{"line_number":273,"context_line":"                        for k, v in i.meta.items()]):"},{"line_number":274,"context_line":"                    # soft-fail"},{"line_number":275,"context_line":"                    # self.log.error(\"Invalid metadata for %s; ignored\""},{"line_number":276,"context_line":"                    #               % i.name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e6f63c0d_dd30ec75","side":"PARENT","line":273,"range":{"start_line":272,"start_character":19,"end_line":273,"end_character":53},"in_reply_to":"54b45231_94b15d2f","updated":"2022-05-23 16:37:52.000000000","message":"It\u0027s unclear from the documentation if the 255 limit is for both the key and the value combined, or the key and value separately; FWIW I\u0027d prefer to just pass it to glance and let it deal with the validation instead of try to second guess it (even on the length), if that\u0027s OK with you.","commit_id":"5551f99ca2aa08aba566f463d1ffa86344e61b35"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"57e000ce4e075c6e48614a637352c1c8b3fa5ad4","unresolved":true,"context_lines":[{"line_number":272,"context_line":"                   any([len(k) \u003e 255 or len(v) \u003e 255"},{"line_number":273,"context_line":"                        for k, v in i.meta.items()]):"},{"line_number":274,"context_line":"                    # soft-fail"},{"line_number":275,"context_line":"                    # self.log.error(\"Invalid metadata for %s; ignored\""},{"line_number":276,"context_line":"                    #               % i.name)"},{"line_number":277,"context_line":"                    i.meta \u003d {}"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"        for image in self.provider.get(\u0027cloud-images\u0027, []):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffe20de3_a6f7b05b","side":"PARENT","line":276,"range":{"start_line":275,"start_character":20,"end_line":276,"end_character":45},"updated":"2022-05-23 03:33:10.000000000","message":"This appears to have been commented out with code movement in I3d1ae0f0a6cc6675169974dd1beece8429071a49.  I do not think that was intentional","commit_id":"5551f99ca2aa08aba566f463d1ffa86344e61b35"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"57e000ce4e075c6e48614a637352c1c8b3fa5ad4","unresolved":true,"context_lines":[{"line_number":274,"context_line":"                    # soft-fail"},{"line_number":275,"context_line":"                    # self.log.error(\"Invalid metadata for %s; ignored\""},{"line_number":276,"context_line":"                    #               % i.name)"},{"line_number":277,"context_line":"                    i.meta \u003d {}"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"        for image in self.provider.get(\u0027cloud-images\u0027, []):"},{"line_number":280,"context_line":"            i \u003d ProviderCloudImage()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1bc447cf_0bb5ac18","side":"PARENT","line":277,"range":{"start_line":277,"start_character":19,"end_line":277,"end_character":31},"updated":"2022-05-23 03:33:10.000000000","message":"overall my minor concern here is that previously if you had a long key (and maybe value) it would be ignored here, and things would be working.  once we gave an error -- per the prior comment it seems we\u0027re not even giving that now -- but it was then ignored.\n\nThere is a *minuscule* chance that this breaks something that was working.  We should probably just call out explicitly in the changelog \"previously metadata was checked in nodepool and invalid values would be silently ignored, now it will be passed to the image service directly and cause an API error\" or similar.","commit_id":"5551f99ca2aa08aba566f463d1ffa86344e61b35"}],"releasenotes/notes/remove-diskimage-meta-checks-90f705903000eece.yaml":[{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"4e6ea7ffa731b91f6b242e3897df67018a841a93","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Removes diskimage.meta checks from the OpenStack driver. The limit of only"},{"line_number":5,"context_line":"    5 entries is anacronistic and now removed. Rather than trying to pre-guess"},{"line_number":6,"context_line":"    what OpenStack wants the metadata is now passed as-is and OpenStack will"},{"line_number":7,"context_line":"    reject it at upload time."}],"source_content_type":"text/x-yaml","patch_set":2,"id":"b4e080db_0487cae4","line":5,"updated":"2022-05-20 18:21:16.000000000","message":"Nit: \"anacHronistic\"","commit_id":"19ef1a096ff3f77069448f6bd23e874064814ddd"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"d38533455ca2e10f53a7fee0a4c289918a2bc3ca","unresolved":false,"context_lines":[{"line_number":10,"context_line":"    all metadata to be silently ignored. Now, metadata will be passed directly"},{"line_number":11,"context_line":"    to glance, and an API error will occur. This may mean that images that"},{"line_number":12,"context_line":"    previously uploaded (with no metadata) will now cause an API error when"},{"line_number":13,"context_line":"    uploading."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"6783c83d_4192b901","line":13,"updated":"2022-05-23 17:15:16.000000000","message":"I guess Ian\u0027s prior question is whether Glance will actually return an error vs silently discarding the non-conforming entries (but the prior implementation in Nodepool was silently discarding them, so I\u0027m not sure that\u0027s effectively a regression even if so).","commit_id":"a6bb2fff426ebc8d031ec992afe4f1a8547305ee"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"43c8dcf0808b5301b4068b148bf62dd346e2b5fd","unresolved":false,"context_lines":[{"line_number":10,"context_line":"    all metadata to be silently ignored. Now, metadata will be passed directly"},{"line_number":11,"context_line":"    to glance, and an API error will occur. This may mean that images that"},{"line_number":12,"context_line":"    previously uploaded (with no metadata) will now cause an API error when"},{"line_number":13,"context_line":"    uploading."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"b5e296d2_ffac152c","line":13,"in_reply_to":"6783c83d_4192b901","updated":"2022-05-24 00:34:34.000000000","message":"Yeah I think that it will reject any keys that are \u003e 255 with appropriate http errors; before they would have been filtered here.  given how unlikely this is to actually cause anyone a regression, just the comment seems sufficient.","commit_id":"a6bb2fff426ebc8d031ec992afe4f1a8547305ee"}]}