Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2019-08-29
16:35:14 elmiko yeah, i'm gonna carve out like half a day for going reviewing these
16:35:28 dtantsur I've seen gtema_'s comments, just never got to fixing them
16:35:36 elmiko ok, maybe i can help
16:35:41 dtantsur and again, I'd prefer to fix in a follow-up
16:35:49 elmiko ++
16:36:02 elmiko if there are easy fixes though, i will try to just add them
16:36:15 gtema_ cool
16:42:43 gtema_ dtantsur: what do you think about https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/resource.py#L1331
16:42:56 gtema_ can we change it to "Accept: *"?
16:43:35 gtema_ empty value is somehow not RFC-nice
16:46:13 dtantsur gtema_: I wonder why we send *anything* if we don't expect a body
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

Earlier   Later