| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2019-08-29 | |||
| 16:46:22 | dtantsur | HEAD + Accept doesn't make sense to me | |
| 16:46:30 | gtema_ | well, some APIs might return a body | |
| 16:46:37 | dtantsur | to HEAD?? Oo | |
| 16:47:16 | gtema_ | or, no | |
| 16:47:22 | gtema_ | looked to the wrong section | |
| 16:47:53 | gtema_ | I would suggest either remove it completely, or set a "*" | |
| 16:47:57 | gtema_ | not an empty value | |
| 16:48:13 | edleafe | A HEAD with a body is, well, a whole person | |
| 16:48:30 | gtema_ | yupp | |
| 16:48:40 | elmiko | lol | |
| 16:49:06 | dtantsur | :D | |
| 16:49:10 | dtantsur | I'd remove it | |
| 16:49:18 | gtema_ | ok, will do. | |
| 16:49:20 | dtantsur | and see if mordred complains | |
| 16:49:46 | gtema_ | and now PUT, which might return body, but contain no body in request (swift create object) | |
| 16:50:02 | dtantsur | Accept is related to returned body, right? | |
| 16:50:04 | elmiko | dtantsur: the ultimate test ;) | |
| 16:50:10 | gtema_ | yes | |
| 16:50:15 | dtantsur | so if you expect that a body might be returned, you should be sending Accept | |
| 16:50:25 | dtantsur | (although the exactly value depends on the call) | |
| 16:50:42 | gtema_ | yeah, but not the "Accept: ", at least "Accept: *" or "Accept: application/json" | |
| 16:50:52 | dtantsur | true | |
| 16:50:59 | dtantsur | probably the default should be Accept:* or no accept | |
| 16:51:00 | gtema_ | this empty value is what breaks it for me | |
| 16:51:12 | dtantsur | then everything except for swift should use Accept: application/json | |
| 16:51:13 | timburke | fwiw, requests adds an "Accept: */*" header by default. if i had to guess, i'd say the empty was trying to prevent that, but i don't know why | |
| 16:51:26 | dtantsur | weird | |
| 16:52:15 | gtema_ | my small test was not confirming that - after removing this empty there was nothing at all | |
| 16:52:18 | timburke | if you really want to drop the header entirely, you could try {'Accept': None} | |
| 16:52:35 | dtantsur | I'm not sure why we would want that | |
| 16:52:52 | dtantsur | I would expect that no header is equivalent to Accept:*/* | |
| 16:53:00 | gtema_ | yeah | |
| 16:53:19 | gtema_ | this is also how I interpret RFC | |
| 16:53:19 | timburke | i know that i've done that for Accept-Encoding before, where swiftclient will freak out a bit if requests goes and decompresses object data as it's getting downloaded | |
| 16:53:35 | dtantsur | gtema_: propose a fix and see if anybody complains | |
| 16:53:42 | gtema_ | cool | |
| 16:53:48 | edleafe | "Accept: None" should mean "don't return *any* body in the response" | |
| 16:53:48 | dtantsur | timburke: but it's swiftclient specific, right? not openstacksdk? | |
| 16:53:56 | gtema_ | exactly SDK | |
| 16:54:12 | elmiko | i don't want to break up the accept discussion too much, but before we end the office hour i /do/ want to thank edleafe one more time. he has been a pillar of this community and i personally have learned a great deal from him. thank you again Ed =) | |
| 16:54:29 | gtema_ | yeah, thanks Ed | |
| 16:55:03 | edleafe | I am going to miss hanging out with all of you. Even elmiko! | |
| 16:55:10 | elmiko | hehehe <3 | |
| 16:55:40 | dtantsur | thanks you edleafe! | |
| 16:55:40 | timburke | i'm not sure what you guys do when downloading object data ;-) i know swiftclient wants to do some checksumming, which'll require the raw (compressed, if it was stored compressed) stream | |
| 16:55:57 | dtantsur | good question | |
| 16:56:47 | gtema_ | there is hell amount of logic | |
| 16:57:12 | gtema_ | at least for uploading | |
| 16:58:05 | gtema_ | for downloading we also set "Accept: bytes" | |
| 17:01:17 | gtema_ | but we do not do download checksumming | |
| 17:01:52 | elmiko | i'm heading out for the weekend, take care all o/ | |
| 17:02:18 | gtema_ | yeah, EOD for me as well. See ya tomorrow | |
| 17:02:20 | timburke | gtema_, curious! good thing swift doesn't do anything with that. we'd 400 it for container listings, complaining "Invalid Accept header" | |
| 17:03:43 | gtema_ | timburke: thanks for hint. Will try to check that tomorrow | |
| 23:24:53 | openstackgerrit | Brin Zhang proposed openstack/python-openstackclient master: Microversion 2.77: Support Specifying AZ to unshelve https://review.opendev.org/665336 | |
| 23:51:59 | openstackgerrit | Brin Zhang proposed openstack/python-openstackclient master: Microversion 2.77: Support Specifying AZ to unshelve https://review.opendev.org/665336 | |
| #openstack-sdks - 2019-08-30 | |||
| 07:21:10 | openstackgerrit | Artem Goncharov proposed openstack/openstacksdk master: Remove Accept header with empty value for HEAD and DELETE requests https://review.opendev.org/679383 | |
| 07:22:38 | gtema | dtantsur, mordred: please please please my stat change | |
| 08:08:32 | mordred | gtema: re: accept headers - I think the default value of '' is weird and it seems like changing that isn't a bad idea | |
| 08:08:49 | gtema | I'm glad | |
| 08:10:12 | gtema | mordred: can you please have another look on stats change? After your +2 I only addressed dtantsurs review - no functional changes | |
| 08:10:39 | gtema | I'm just tired rebasing my dockerfile, where I checkout this change | |
| 08:10:54 | mordred | gtema: yup. just re-reviewed | |
| 08:11:04 | gtema | cool, thanks | |
| 08:11:43 | mordred | gtema: does that accept patch work on your cloud? I though someone was saying requests adds a */* if you don't do anything and you needed to pass accept: None if you wanted no header ... | |
| 08:11:54 | mordred | (although it looks like the unit tests show no header - so I guess it does) | |
| 08:12:17 | gtema | well, with this I was only only checking the Swift - and it works this way | |
| 08:12:38 | gtema | Actually even swiftclient if you look is not passing any "Accept" header | |
| 08:13:03 | gtema | but since I modified HEAD and DELETE globally - this might bring problems somewhere | |
| 08:13:32 | mordred | I doubt it... but I guess anything is possible :) | |
| 08:13:40 | dtantsur | I think we should start with being standard compliant, and only add hacks if proved required | |
| 08:13:46 | gtema | that's exactly the problem - everything is possible | |
| 08:13:58 | mordred | dtantsur: ++ | |
| 08:14:37 | gtema | dtantsur: now I definitely owe you a beer. Let's see when we can meet | |
| 08:14:43 | dtantsur | ++ :) | |
| 08:15:24 | gtema | btw, I hope in one-two weeks I plan to come back to switching remaining stuff in cloud to proxy/resource | |
| 08:15:48 | gtema | but we need to address one thing - caching | |
| 08:17:00 | gtema | it's either we still cast everything to dict in the cloud return, or we teach dogpile/etc to serialize Resource (what it can't do now) | |
| 08:18:53 | gtema | dtantsur: I also answered your question in https://review.opendev.org/#/c/665110/ - it's a default microver, so no change is required | |
| 08:19:23 | mordred | gtema: oh - yeah - I have some thoughts about that - I should write them down so we can discuss whether they're any good | |
| 08:19:36 | gtema | cool | |
| 08:21:39 | dtantsur | gtema: approved as well | |
| 08:24:08 | gtema | thanks | |
| 09:32:34 | openstackgerrit | Merged openstack/openstacksdk master: Rework statistics reporting https://review.opendev.org/659841 | |
| 09:39:26 | openstackgerrit | Bo Tran proposed openstack/openstacksdk master: Add a fields meta_data to result of Senlin API https://review.opendev.org/678585 | |
| 12:17:37 | frickler | mordred: dtroyer: I'm still seeing this "location | Munch..." spam in osc output, I thought we had fixed that some time ago already. can we get that fixed for the upcoming release still? http://paste.openstack.org/show/767631/ | |
| 12:24:02 | frickler | the issue is triggered by openstacksdk>=0.24.0 but I can't remember whether it needs fixing in sdk or on osc side | |
| 12:27:45 | Shrews | frickler: yeah, that's less than pleasing. I'm guessing it should be osc side since sdk is supposed to return Munch objects to the callers | |
| 12:30:01 | dtroyer | frickler: one of the things we pushed in for osc4 is the output formatting changes, it may have crept back in with that sinne some of those were rather old | |
| 12:30:41 | dtroyer | the good news there is that it might only be ne place to change… I do not recall what the fix was initially | |
| 12:32:08 | gtema_ | dtroyer, since you are here and I remember this. I am going to finally come back to glanceclient=>SDK replacement (I think I need to redo this from scratch, rather then try to rebase). | |
| 12:32:10 | frickler | dtroyer: it seems to happen with all 3.x released version I've tested, so possibly it never got fixed | |
| 12:32:35 | gtema_ | should I do this in osc4 branch or in master? | |
| 12:34:41 | dtroyer | gtema_: the osc4 branch is dormant. I rebased what we had into back into master after the last release so osc4 is in master now and will be the next release | |
| 12:34:56 | gtema_ | ah, cool. | |
| 12:36:36 | dtroyer | frickler: ah, ok. next Q then is if it happens only in columns with the new formatting or olny not in those. I've seen it but not kept track of exactly where… | |
| 12:45:12 | frickler | dtroyer: I'm not sure I understand that question. from a quick test up to now I only see this issue for command showing/creating items living in the neutron api | |
| 12:50:51 | dtroyer | frickler: that is probably true as the others do not use SDK | |
| 12:52:04 | dtroyer | the change is the introduction of osc_lib.format_columns module to de-pythonize structured output, a recent example is on https://review.opendev.org/#/c/678689/2/openstackclient/compute/v2/aggregate.py | |
| 12:52:40 | dtroyer | without looking I am hoping we can just make DictColumn understand Munch | |