proxy.controllers.base should use swob.HeaderKeyDict

[clayg]

We do a bunch of work to normalize case in swift.proxy.controller.base - but it'd be easier to reason about and less error prone to just use a case insensitive dictionary like swob.HeaderKeyDict consistently.

Specifically it could be applied cleanly in _prep_headers_to_info

[Ben Keller (bjkeller)]

It seems HeaderKeyDict returns everything as a string, but some unit tests are expecting integers. Would it be best to change numbers back to ints in _prep_headers_to_info or just pass everything as a string?

[clayg (clay-gerrard)]

reading around proxy.controllers.base I'm worried we're not consistently converting headers representing numerical values to integers across the board for all *_info keys. And in fact *can not* guarantee we do so in the general sense (user-meta, new info keys added). We could try to be very disciplined about the practice, but I'm not sure to what value - a defensive user of the *_info is going to cast any values coming out of the cache that it needs to be an integer - to that type anyway [1].

Maybe it would be better to just consistently make the type of the *_info values be strings - that's what they started as in most (all?) cases when they came off the wire. Plus you get a common expectation - all the values in *_info are strings unless you change them - seems easy to reason about. If some consumer of *_info values needs them to be a different type for some reason - let the code that's using it in context change the type - closer to what it needs to ensure type correctness - not some distant far away part of the system that's probably going to get it wrong half the time and have a loose contract of "sometimes int-ish values might be integers instead of strings unless I forgot".

But that's just like *my* opinion man. Try it both ways and see which diff you like better. If it's a big mess to go strings all the time don't do it. Another option might be to extract most of HeaderKeyDict into a base CaseInsenstiveDict and leave the weird type contortion in HeaderKeyDict - overridden in it's __setitem__ method.

Feel free to push up the diff of what you have so far and ping me (or any other swift junkie) in IRC (#openstack-swift in freenode). You can set the patch in jenkins to "Work in Progress" and we can still see it and give you feedback directly on the code even before you think it's "done" and "ready for review".

GL!

1. e.g. https://github.com/openstack/swift/blob/5ec586bd346a9b36dbba450f37c891dfa190b5da/swift/common/middleware/account_quotas.py#L124

[Ben Keller (bjkeller)]

The tempest gate check keeps failing, calling a mismatchError on the etag. Could this have to do with sending headers as strings?

[OpenStack Infra (hudson-openstack)]

Change abandoned by Janie Richling (<email address hidden>) on branch: master
Review: https://review.openstack.org/274906
Reason: Abandoning due to the fact that Ben Keller no longer works on swift, and Bryan Keller is now assigned the associate bug to fix.

[clayg (clay-gerrard)]

Expanding on my comment in #2 - get_container_info and get_object_info and get_account_info have some non-sense like this at the bottom:

https://github.com/openstack/swift/blob/master/swift/proxy/controllers/base.py#L355

IMHO that should *probably* not exist - it depends on how many places are relying on that type conversion. If it *should* exist (i.e. it turns out way too many places assume it so) - then it should should exist in way that has some good properties

a) there's only one list to track down all of the keys that have this magic type conversion behavior

b) when I i'm working with code that has a dict-like thing which *un*-like a a regular dict of headers has this magic property that *some* are integers I can tell this by looking at the *type*

i.e. Make a thing like "MagicIntMakingHeaderLikeDict"

... but ideally we could just use a case insensitive dict for header-like things and the values would be strings until some code working with them needs them to be an int and converts them in context. I think.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/410407