opendevreview | Rajat Dhasmana proposed openstack/glance-specs master: Update new location APIs spec https://review.opendev.org/c/openstack/glance-specs/+/883491 | 10:01 |
---|---|---|
whoami-rajat | dansmith, i can't agree more, I have been working on this location thing since Yoga and would really like to move on to something else, will update with your suggestion | 14:02 |
dansmith | whoami-rajat: ack, let's get the spec merged.. I haven't been able to overlap with pdeore, but I hope she's working on the implementation and want to make sure she gets help if needed so we don't slip on this | 14:03 |
dansmith | (the spec update I mean) so that it's not looking like it's still ill-defined | 14:04 |
dansmith | afaict, we're pretty well settled on the meat of it and just tweaking the result is pretty minor | 14:04 |
whoami-rajat | dansmith, yep, currently taking the cinder meeting, will update the spec after that | 14:07 |
dansmith | ++ thanks | 14:07 |
rosmaita | whoami-rajat: dansmith: ping me for a quick review when it's been updated | 14:17 |
opendevreview | Rajat Dhasmana proposed openstack/glance-specs master: Update new location APIs spec https://review.opendev.org/c/openstack/glance-specs/+/883491 | 14:24 |
whoami-rajat | dansmith, rosmaita does this look good? let me know if another update is needed and will quickly do it ^ | 14:24 |
dansmith | whoami-rajat: it needs to be the same schema, so those os_hash_* things need to be inside validation_data | 14:26 |
dansmith | and also, you won't get the checksum back from a request that provided validation data because it will only calculate it async | 14:27 |
dansmith | also I don't think we ever need checksum in there because it's on the image.. this is just what you provided for the location | 14:27 |
whoami-rajat | dansmith, oh yeah, correct, will update it | 14:28 |
dansmith | generally you should be able to apply the same schema def/checking to the request and the response | 14:29 |
whoami-rajat | dansmith, but we are providing checksum in the request, what i understand is we don't want to return the checksum until it's validated but doesn't that also hold true for hash value? | 14:30 |
dansmith | whoami-rajat: oh sorry right.. I missed that that's in the request.. I'm confused why we're passing os_hash and checksum in request? | 14:32 |
whoami-rajat | dansmith, also hash value is for the image as well | 14:32 |
whoami-rajat | dansmith, to validate it against the glance calculated values? | 14:32 |
whoami-rajat | isn't that the purpose of validation data? | 14:32 |
dansmith | sorry just a sec, joining a call | 14:33 |
whoami-rajat | ack np | 14:34 |
dansmith | I had thought we were only taking os_hash_algo and os_hash_value and not checksum in the request, because we were going to use os_hash_algo being set to indicate that it's in progress | 14:36 |
dansmith | checksum is what, md5? | 14:36 |
whoami-rajat | dansmith, yes, md5 | 14:36 |
dansmith | I think it would be better to just accept os_hash_algo and os_hash_value and not checksum, because it's more straightforward..we will still calculate checksum (I assume) but it seems like we might as well not accept both just to avoid confusion | 14:37 |
dansmith | rosmaita: what do you think? | 14:37 |
whoami-rajat | dansmith, this is the format of validation data and it has 3 fields including checksum https://specs.openstack.org/openstack/glance-specs/specs/stein/implemented/glance/spec-lite-locations-with-validation-data.html | 14:37 |
dansmith | all the discussion above about procedure doesn't mention checksum at all, which is why I didn't notice it was in the request | 14:38 |
dansmith | whoami-rajat: yeah, it's okay to calculate and expose it, I just wouldn't worry about accepting it in the request | 14:38 |
rosmaita | do we still compute the checksum? | 14:39 |
dansmith | idk, if we do then fine, but let's not take it as input, IMHO | 14:39 |
dansmith | and I'm not even sure we need to show it on the location after it's calculated.. just on the image should be fine | 14:39 |
whoami-rajat | ok, so no checksum in request or response? | 14:40 |
dansmith | that's my preference | 14:41 |
whoami-rajat | ok, i will update it and see if others have a different view about it | 14:42 |
opendevreview | Rajat Dhasmana proposed openstack/glance-specs master: Update new location APIs spec https://review.opendev.org/c/openstack/glance-specs/+/883491 | 14:54 |
whoami-rajat | dansmith, how does this look?^ i had to remove some other occurrences of checksum | 14:55 |
rosmaita | whoami-rajat: dansmith: i didn't realize this was going to be so complicated, i'll have to read through the scrollback and think a bit when reviewing | 14:55 |
whoami-rajat | rosmaita, i can summarize, | 14:57 |
whoami-rajat | 1) changed the response from whole image to location object | 14:57 |
whoami-rajat | 2) removed checksum from request response | 14:57 |
rosmaita | ok, thanks, helps me know what to focus on | 14:58 |
dansmith | honestly, it's just what we spent all the time discussing in terms of what to provide and expect for hashingm, | 15:04 |
dansmith | the request/response examples just didn't get updated | 15:04 |
dansmith | whoami-rajat: yes, that version looks good to me | 15:04 |
whoami-rajat | ack, thanks | 15:07 |
opendevreview | Cyril Roelandt proposed openstack/python-glanceclient master: Docs generation: mock the six module for autodoc https://review.opendev.org/c/openstack/python-glanceclient/+/885505 | 15:27 |
dansmith | rosmaita: can we get this +2 for pranali to hopefully hit in the morning? https://review.opendev.org/c/openstack/glance-specs/+/883491 | 20:41 |
rosmaita | dansmith: completely forgot about it, will look now | 20:41 |
dansmith | thanks | 20:42 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!