21:00:16 <notmyname> #startmeeting swift
21:00:17 <openstack> Meeting started Wed Jan 18 21:00:16 2017 UTC and is due to finish in 60 minutes.  The chair is notmyname. Information about MeetBot at http://wiki.debian.org/MeetBot.
21:00:18 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
21:00:20 <openstack> The meeting name has been set to 'swift'
21:00:32 <notmyname> who's here for the swift team meeting?
21:00:34 <timburke> o/
21:00:36 <m_kazuhiro> o/
21:00:41 <cschwede> o/
21:00:42 <kota_> hello
21:00:44 <bkeller`> o/
21:00:45 <mathiasb> o/
21:00:47 <tdasilva> yo
21:01:41 <notmyname> welcome everyone
21:01:53 <notmyname> looks like we're a little lower in attendance today
21:02:12 <notmyname> but that's ok. just means we'll get done sooner ;-)
21:02:20 <mmotiani1> Hi
21:02:26 <notmyname> topics for this week, as always are at https://wiki.openstack.org/wiki/Meetings/Swift
21:02:27 <notmyname> #link https://wiki.openstack.org/wiki/Meetings/Swift
21:02:54 <notmyname> there's one new topic, and like the past couple of weeks, I'd like to catch up on the different ongoing things
21:02:59 <hurricanerix> o/
21:03:26 <notmyname> first up, https://bugs.launchpad.net/swift/+bug/1254638
21:03:26 <openstack> Launchpad bug 1254638 in OpenStack Object Storage (swift) "Some bad parameters do not return 4xx" [Wishlist,In progress] - Assigned to Ankur Jain (j-ankur)
21:03:39 <notmyname> timburke: what's the story? what's the question? what needs to be discussed?
21:04:48 <timburke> so, currently on master if you send a request to an account or container server with query params like ?limit=foo, we just ignore it and return up to account/container_listing_limit
21:05:34 <notmyname> so if you send an invalid parameter, we ignore it?
21:06:23 <timburke> there's a patch to make that do something reasonable -- enforce the currently documented requirement that you send an integer, 400'ing if you send something invalid -- but clayg was worried that it would break existing clients
21:06:31 <cschwede> wait a second - send a query to a account/container *server*?
21:06:53 <notmyname> including if you send a request for a too-big number? (eg limit=1000000 if the max is the default of 10000)
21:07:18 <cschwede> you mean the request gets forwarded with that parameter from proxy to account/container server, right?
21:07:19 <acoles> sorry I was late
21:07:22 <timburke> cschwede: yes; like delimiter, marker, etc. the query params from the client get passed directly through the proxy to the account/container server
21:07:49 <cschwede> ah, ok. got it. i thought i might have misunderstood and someones sends direct request to acc/cont servers
21:08:16 <timburke> notmyname: no, that 412s: https://github.com/openstack/swift/blob/master/swift/container/server.py#L480-L484
21:08:41 <timburke> (despite decidedly *not* involving preconditions...)
21:09:28 <timburke> cschwede: the patch as it stands *would* still affect direct clients, however. that was actually one of the concerns i had with the previous patchset
21:09:28 <notmyname> looking at that code, I wonder what a limit=-1 does...
21:09:51 <notmyname> timburke: which patch?
21:09:53 <timburke> iirc, the same as limit=0
21:10:21 <timburke> notmyname: https://review.openstack.org/#/c/299225/
21:10:38 <notmyname> so the proposal is to change from ignoring non-ints in limit to returning an error to the client
21:10:42 <notmyname> right?
21:10:57 <timburke> yup.
21:12:07 <timburke> clayg felt that the "needs-new-api" tag on the bug was warranted; i felt like we can reasonably tell broken clients that they're broken
21:12:26 <notmyname> that means a client thatis currently getting a 200 response will start getting a 4xx with no changes if this patch lands
21:13:01 <jrichli> sorry im late
21:13:02 <cschwede> I like the 2013 comment from clayg in this case - "But i like the python philosophy of never silence errors unless explicitly silenced so I think a BadRequest would be better."
21:13:24 <cschwede> even if that means we fix aka change the api in some way
21:13:45 <timburke> notmyname: ...and they're currently sending things like ?limit=one and expecting it to work, yes. only it doesn't work as intended anyway
21:13:55 <notmyname> I mean, yeah, it's pretty safe to assume that a client sending limit=not_integer is doing something wrong
21:13:57 <timburke> (well, as the client intended)
21:14:17 <notmyname> but the fact is that we'll break clients in that a currently successful response will start getting 4xx responses
21:14:59 <cschwede> notmyname: well, the result is wrong atm, but a client might not recognize this - which sounds wrong too?
21:15:07 <timburke> ...do we know of any clients *expecting* the current behavior?
21:15:48 <notmyname> cschwede: only in that the "correct" result is undefined? we can't say what the correct response is anything in particular with the given input, right?
21:15:48 <timburke> this seems to me like we're playing fast & loose with user input validation
21:16:03 <timburke> ...which is pretty much *never* a good thing
21:16:24 <notmyname> that seems to me to be what the whole meaning of 4xx is: the server can't determine what the correct response is based ont eh input, so returns a client error
21:16:42 <notmyname> but in this case, we are currently actually responding with a 200 today and we'll start returning a 400
21:16:45 <notmyname> that doesn't seem right
21:17:08 <notmyname> despite the fact that *surely* none of us would ever write bugs in our code that send bad parameters ;-)
21:17:30 <timburke> yeah, clay mentioned the fragment-index:None bug
21:17:38 <cschwede> notmyname: ?limit=-1 sounds wrong *to me*, but yes - that depends on someones view. so forcing clients to make an explicit choice sounds somewhat reasonable to me
21:17:59 <notmyname> is itsimilar to the HTTP range spec which says that if the range value is unparseable, it's ignored?
21:18:08 <timburke> seems like if we'd validated our expectations and 400ed the request it would've been caught sooner?
21:19:04 <timburke> iirc there's no guidance
21:19:22 <notmyname> so what's the problem with keeping it the way it is today? what is it that's broken? is it just that we are being explicit about the fact that the client's request doesn't make sense?
21:19:55 <notmyname> (doesn't make sense...despite the fact that we've been returning a 200 for that same request for years)
21:20:54 <notmyname> I'm wondering what others thing. acoles? kota_? jrichli?
21:21:16 <timburke> and to save some future client developer from wasting time debugging and cursing swift for returning more than 100 results when they clearly told it to... wait why's that null?
21:21:21 <acoles> aren't there other examples of "bad" request params that get ignored e.g. ?format=flobalob for a listing?
21:21:49 <acoles> ?multipart-manifest=foo
21:22:05 <cschwede> looking at the patch, there are some parameters that might be wrong, but shouldn't change - eg "?limit="
21:23:06 <timburke> ?delimiter=foo is a firm error https://github.com/openstack/swift/blob/master/swift/container/server.py#L470-L472
21:24:42 <jrichli> I dont have an opinion yet.  will need to take a look after the meeting and gather thoughts
21:25:22 <notmyname> so we're relatively inconsistent (and could probably spend a whole day arguing about which inconsistencies are right). meaning that both "sides" could lean on precedent
21:25:45 <timburke> yup :-)
21:26:43 <matt6434> soorry I'm late, took me a while to hobble into the conference venue
21:27:31 <timburke> but it probably isn't worth arguing for too long; we've got other things on the agenda... i really didn't think it'd be a 20+ min topic
21:27:38 <notmyname> lol
21:27:46 <hurricanerix> +1 to wait for some new version of the api that fixes stuff like this and makes the overall api more consistent.
21:27:55 <notmyname> timburke: have you ever *been* around devs discussing APIs?
21:28:04 <timburke> hurricanerix: that day will never come :P
21:28:14 <hurricanerix> hehe
21:28:36 <hurricanerix> timburke maybe if we write in more inconsistency it will...
21:28:38 <acoles> so I'm being a little pedantic but the API doc says for ?limit "For an integer value n , limits the number of results to n ." i.e. it does not say it must be an int, just says what will happen if it is an int.
21:28:42 <notmyname> yeah, it might never come, but I too am leaning towards the "needs-new-api" tag staying on the bug
21:29:17 <acoles> ok so I'm leaning on interpretation of English language to justify me sitting on the fence!
21:29:34 <notmyname> "undefined"
21:30:34 <notmyname> so to resolve this for today's meeting, I don't think we can move forward with the patch yet
21:31:08 <notmyname> shall we think on it more and come back to it next week?
21:31:12 <timburke> i don't like undefined behavior. it leads to strange places, like https://blogs.msdn.microsoft.com/oldnewthing/20140627-00/?p=633/
21:31:24 <notmyname> or -2 the patch and call it done?
21:31:40 <timburke> eh, i'll be content to let it stand, but we should close out the patch
21:32:22 <notmyname> ok
21:32:31 <notmyname> timburke: would you be ok with adding a -2, or would you prefer that I do?
21:32:42 <acoles> (don't mistake this for me defending the current behaviour) we could update the api ref to say non-int value is ignored.
21:32:50 <timburke> i'll abandon. pretty sure the original author already has
21:32:57 <notmyname> timburke: ok, thanks
21:32:59 <notmyname> acoles: definitely
21:33:01 <timburke> acoles: we definitely should
21:33:13 <acoles> that involves yaml :/
21:33:16 <timburke> and add tests for that behavior
21:33:19 <acoles> JK
21:33:28 <kota_> +1
21:33:36 <notmyname> ok, moving on, then
21:33:44 <notmyname> #topic status updates
21:33:58 <notmyname> acoles: what's going on with the https://bugs.launchpad.net/swift/+bug/1651530 bug?
21:33:58 <openstack> Launchpad bug 1651530 in OpenStack Object Storage (swift) "suffix hash invalidation may be lost" [Critical,New]
21:35:02 <acoles> ok so I have started reviewing clayg's set of alternative mini-patches
21:35:23 <acoles> only one of which I guess addresses that bug, and I haven't got to that one yet.
21:35:35 <clayg> acoles: i'm sorry i didn't update them yesterday - i am *very* sick :'(
21:35:36 <acoles> but we'd probably agree the performance regression is more critical
21:35:38 <kota_> #link https://review.openstack.org/#/c/418690/
21:35:45 <kota_> that one
21:35:48 <acoles> clayg: get well first!
21:35:50 <clayg> but while lying in bed I was *thinking* about get_hashes the whole time
21:36:00 <acoles> clayg: that is why you are sick
21:36:01 <notmyname> clayg: feel better (and go back to bed) ;-)
21:36:41 <acoles> I am definitely now liking separate mini-patches, I get smaller headaches with them :)
21:36:43 <kota_> clayg: please get well
21:36:59 <acoles> and I think I maybe persuaded jrichli to join in reviewing
21:37:06 <notmyname> great!
21:37:11 <notmyname> thanks everyone for working on those
21:37:22 <jrichli> i can probably tell from the patches themselves, but could you give me the patch number for the one that fixes the critical bug?
21:37:37 <notmyname> jrichli: the one kota_ linked
21:37:48 <jrichli> oh, thx.  i am a little distracted today
21:38:47 <notmyname> cschwede: what's the progress this week on the part power increase patch?
21:38:52 <acoles> so for anyone else...go to https://review.openstack.org/#/c/418689/4 and there are a bunch of related patches listed there, they all depend on 418689 which is just test infratructure
21:39:30 <cschwede> notmyname: mattoliverau helped with reviews, i updated the patch based on his comments - still has a +2 from Pete
21:39:42 <notmyname> nice!
21:39:47 <mattoliverau> cschwede: I'll finish looking at it today
21:39:56 <notmyname> kota_: how's the global ec patch going?
21:39:58 <cschwede> mattoliverau: thx a lot Matt!
21:40:27 <pdardeau> cschwede: i haven't reviewed yet, hopefully on Friday
21:40:28 <zaitcev> The part++ patch looks fine to me. Well, I missed those floats last time I put a +2 on it.
21:40:51 <zaitcev> The cancellation I'm not particularly hot about but I'm okay with it.
21:42:14 <kota_> notmyname: ah
21:42:29 <kota_> notmyname: global ec had good progress in this week, i think
21:42:49 <mattoliverau> zaitcev: fair enough, thanks :)
21:42:50 <notmyname> good :-)
21:42:55 <kota_> clayg and timburke added worthful comments and I addressed almost of them
21:43:01 <timburke> i'm still a bit worried about the possibility of getting a lumpy distribution, with all of the copies of a fragment archive in a single region, for example. i'm also not sure of a good way to address that, though :-/
21:43:34 * notmyname has to log off in a few minutes so .....
21:43:40 <kota_> only one thing i'd mention, i made a separate patch for it
21:43:45 <kota_> #link https://review.openstack.org/#/c/421673/
21:43:47 <notmyname> what other updates are there from ongoing work?
21:43:53 <timburke> kota_: is there the ability to migrate a policy from ec_duplication_factor=1 -> 2, say?
21:44:00 <kota_> eh, it looks pep8 erro though....
21:44:18 <kota_> timburke: no, for now.
21:44:23 <jrichli> tdasilva has been working symlinks task cards like a champ :-)
21:44:28 <tdasilva> symlink update coming soon to a gerrit near you
21:44:36 <notmyname> tdasilva: jrichli: cool. thanks
21:44:36 <kota_> timburke: it should be a brand-new policy as well as existing ones
21:44:40 * tdasilva thinkgs that sounded lame
21:44:45 <mattoliverau> I'm working on merging the latested sharding POC to master, then will push it up as WIP.. there is still alot of work to do, but I think it's time to put it into gerrit for more eyes (and people to fix my bad code)
21:45:26 <notmyname> any major blockers that need to be brought up for this week?
21:46:02 <mattoliverau> notmyname has his talk today.. and it can be streamed live.. for those who don't sleep
21:46:10 <notmyname> lol
21:46:24 <jrichli> what's the topic?
21:46:29 <notmyname> crypto in swift
21:46:34 <jrichli> oh, cool
21:46:43 <acoles> mattoliverau: lol, you mean it may help those who don't sleep, or what?
21:46:54 <notmyname> exactly!
21:47:10 <mattoliverau> ^^ I didn't say it
21:47:28 <acoles> notmyname: it was an innocent question that's all ;)
21:47:33 <notmyname> ok, I need to run to get down to the morning conference activities
21:47:40 <acoles> notmyname: good luck!
21:47:43 <notmyname> thank you for coming, and thanks for working on swift!
21:47:45 <notmyname> acoles: thanks
21:47:48 <notmyname> #endmeeting