{"action":"created","issue":{"url":"https://api.github.com/repos/kevin1024/vcrpy/issues/203","labels_url":"https://api.github.com/repos/kevin1024/vcrpy/issues/203/labels{/name}","comments_url":"https://api.github.com/repos/kevin1024/vcrpy/issues/203/comments","events_url":"https://api.github.com/repos/kevin1024/vcrpy/issues/203/events","html_url":"https://github.com/kevin1024/vcrpy/pull/203","id":103801039,"number":203,"title":"Insensitive headers","user":{"login":"IvanMalison","id":1246619,"avatar_url":"https://avatars.githubusercontent.com/u/1246619?v=3","gravatar_id":"","url":"https://api.github.com/users/IvanMalison","html_url":"https://github.com/IvanMalison","followers_url":"https://api.github.com/users/IvanMalison/followers","following_url":"https://api.github.com/users/IvanMalison/following{/other_user}","gists_url":"https://api.github.com/users/IvanMalison/gists{/gist_id}","starred_url":"https://api.github.com/users/IvanMalison/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/IvanMalison/subscriptions","organizations_url":"https://api.github.com/users/IvanMalison/orgs","repos_url":"https://api.github.com/users/IvanMalison/repos","events_url":"https://api.github.com/users/IvanMalison/events{/privacy}","received_events_url":"https://api.github.com/users/IvanMalison/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/kevin1024/vcrpy/labels/in%20progress","name":"in progress","color":"ededed"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-08-28T21:44:21Z","updated_at":"2015-08-31T16:33:58Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/kevin1024/vcrpy/pulls/203","html_url":"https://github.com/kevin1024/vcrpy/pull/203","diff_url":"https://github.com/kevin1024/vcrpy/pull/203.diff","patch_url":"https://github.com/kevin1024/vcrpy/pull/203.patch"},"body":"WIP. letting people take a look to see if I should abandon or not"},"comment":{"url":"https://api.github.com/repos/kevin1024/vcrpy/issues/comments/136422881","html_url":"https://github.com/kevin1024/vcrpy/pull/203#issuecomment-136422881","issue_url":"https://api.github.com/repos/kevin1024/vcrpy/issues/203","id":136422881,"user":{"login":"agriffis","id":50637,"avatar_url":"https://avatars.githubusercontent.com/u/50637?v=3","gravatar_id":"","url":"https://api.github.com/users/agriffis","html_url":"https://github.com/agriffis","followers_url":"https://api.github.com/users/agriffis/followers","following_url":"https://api.github.com/users/agriffis/following{/other_user}","gists_url":"https://api.github.com/users/agriffis/gists{/gist_id}","starred_url":"https://api.github.com/users/agriffis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/agriffis/subscriptions","organizations_url":"https://api.github.com/users/agriffis/orgs","repos_url":"https://api.github.com/users/agriffis/repos","events_url":"https://api.github.com/users/agriffis/events{/privacy}","received_events_url":"https://api.github.com/users/agriffis/received_events","type":"User","site_admin":false},"created_at":"2015-08-31T16:33:58Z","updated_at":"2015-08-31T16:33:58Z","body":"Hey @IvanMalison \r\n\r\nLots of thoughts here. Let's talk about design, then purpose, then design...\r\n\r\nRegarding design:\r\n  * `DictDecorator` tries to implement a proxy, but it's broken because it doesn't call back to the MRO leaf as proper Python inheritance would. See https://code.activestate.com/recipes/519639-true-lieberman-style-delegation-in-python/\r\n  * `DictDecorator` implements too many methods. The raison d'être of `MutableMapping` is that it's hard to inherit from the built-in `dict` because the implementation takes shortcuts internally rather than calling its own methods, so `MutableMapping` provides an implementation where the subclass only needs to implement a small set of core methods. See https://hg.python.org/cpython/file/tip/Lib/_abcoll.py#l493\r\n\r\nRegarding purpose:\r\n  * I'm definitely on-board with making VCR.py properly preserve multiple header lines, both in the real request and the recorded cassette. It's probably rare in practice, but it's [supposed to mean the same thing as a comma-separated list](http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2)\r\n  * While we're making this effort, we should also preserve the order of all the headers. It's not supposed to matter (ref above) but for the sake of transparency it's worthwhile.\r\n  * We probably want separate matchers, `headers` which compares a dict (with duplicate headers comma-joined!) and `headers_strict` which compares the fully ordered headers.\r\n  * Filters still want a simplified case-insensitive dict-based view of headers with single values, probably exactly like the `headers` matcher above, including comma-joining. (I will also want this to be mutable for #196)\r\n\r\nBack to design:\r\n  * It seems like the underlying storage should actually be a list of KV tuples.\r\n  * Proxies (delegation) are most useful when you need to wrap an instance of something, especially to change the behavior when you have no opportunity to specify a subclass. I'm not sure they're the right answer here, even if implemented properly. It probably makes more sense to implement a `MutableMapping` over the top of the KV list, then have a mixin to implement case normalization, and finally instantiate `HeadersDict` inheriting from both, something like this (off the top of my head, completely untested):\r\n\r\n```python\r\nclass ConcatDict(collections.MutableMapping):\r\n\r\n    def __init__(self, kvlist):\r\n        self._kvlist = kvlist\r\n\r\n    def __getitem__(self, key):\r\n        values = [v for k, v in self._kvlist if k == key]\r\n        if not matches:\r\n            raise KeyError(key)\r\n        return ','.join(values)\r\n\r\n    def __setitem__(self, key, value):\r\n        try:\r\n            i = self._kvlist.find(key)\r\n        except ValueError:\r\n            self._kvlist.append((key, value))\r\n        else:\r\n            del self._kvlist[key]\r\n            self._kvlist.insert(i, (key, value))\r\n\r\n    def __delitem__(self, key, value):\r\n        self._kvlist[:] = [(k, v) for k, v in self._kvlist if k != key]\r\n\r\n    def __iter__(self, key, value):\r\n        return (k for k, v in self._kvlist)\r\n\r\n    def __len__(self):\r\n        return len(self._kvlist)\r\n\r\n\r\nclass CaseInsensitiveMixin(object):\r\n\r\n    def __getitem__(self, key):\r\n        try:\r\n            k, v = super(CaseInsensitiveMixin, self).__getitem__(key.lower())\r\n        except KeyError:\r\n            raise KeyError(key)\r\n        else:\r\n            return v\r\n\r\n    def __setitem__(self, key, value):\r\n        super(CaseInsensitiveMixin, self).__setitem__(key.lower(), (k, v))\r\n\r\n    def __delitem__(self, key):\r\n        super(CaseInsensitiveMixin, self).__delitem__(key.lower())\r\n\r\n\r\nclass HeadersDict(CaseInsensitiveMixin,\r\n                  ConcatDict):\r\n    pass\r\n\r\n\r\nclass Request(object):\r\n\r\n    def __init__(self):\r\n        self._headers = []\r\n\r\n    @property\r\n    def headers(self):\r\n        return HeadersDict(self._headers)\r\n\r\n```\r\n\r\nFinal notes:\r\n  * This is going to change the cassette format, necessitating a major version bump. We'll also want either a converter for old cassette files, or to read them transparently. This is begging for versioned cassette files (unless they're already versioned; I haven't checked).\r\n  * I'd really like to get #196 merged before fully tackling this. That PR is finished except for tweaking docs, and I need it for vcrpy-facebook. It doesn't break compatibility and it would make sense to push out a release after we get it in."}}