Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2019-08-29
16:30:39 efried clear guides written for different times; we should have the temerity to alter those processes.
16:30:48 elmiko fair
16:31:10 dtantsur ++
16:31:34 efried A number of teams are, out of necessity, moving to single-core approvals. Not saying thats' a good idea here necessarily, just that it's a thing.
16:31:36 elmiko appreciate all the feedback, it is helpful
16:31:37 dtantsur and the version discovery one doesn't have -1's
16:31:48 elmiko dtantsur: ++
16:31:50 dtantsur so strictly speaking we can approve it
16:31:59 elmiko cool, maybe we just do that
16:32:16 elmiko give it one more read through to make sure there are not glaring errors
16:32:25 efried Yeah, for something like this, if it's clear it's been thoroughly reviewed by people with at least some understanding of the content, it's reasonable to proxy their +1 as a +2 when there's a dearth of cores available.
16:32:27 efried IMHO
16:32:35 elmiko ++
16:32:47 efried IIRC I went through one or more of those enormous patches and +1ed.
16:32:55 efried I consider myself at least somewhat up on version discovery.
16:33:19 efried and changes since my +1 have been minimal, so feel free to carry those forward.
16:33:27 elmiko ack, thanks
16:33:56 elmiko is it time for cake and punch?
16:34:01 gtema_ I was missing in the version discovery rules on what is open and what requires AUTH. This got lost in some restructuring
16:34:28 elmiko gtema_: definitely add comments if you can figure out where it was
16:34:35 gtema_ did it already
16:34:40 gtema_ long time ago
16:34:55 gtema_ but that is the point - there are way to many to see the overview
16:34:58 elmiko ok, cool. i will inevitably come across them ;)
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 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:19 gtema_ this is also how I interpret RFC
16:53:35 dtantsur gtema_: propose a fix and see if anybody complains
16:53:42 gtema_ cool
16:53:48 dtantsur timburke: but it's swiftclient specific, right? not openstacksdk?
16:53:48 edleafe "Accept: None" should mean "don't return *any* body in the response"
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 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:40 dtantsur thanks you edleafe!
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 ...

Earlier   Later