{"action":"created","issue":{"url":"https://api.github.com/repos/jonparrott/goth/issues/1","repository_url":"https://api.github.com/repos/jonparrott/goth","labels_url":"https://api.github.com/repos/jonparrott/goth/issues/1/labels{/name}","comments_url":"https://api.github.com/repos/jonparrott/goth/issues/1/comments","events_url":"https://api.github.com/repos/jonparrott/goth/issues/1/events","html_url":"https://github.com/jonparrott/goth/issues/1","id":179608376,"number":1,"title":"Internal transport discussion","user":{"login":"jonparrott","id":250995,"avatar_url":"https://avatars.githubusercontent.com/u/250995?v=3","gravatar_id":"","url":"https://api.github.com/users/jonparrott","html_url":"https://github.com/jonparrott","followers_url":"https://api.github.com/users/jonparrott/followers","following_url":"https://api.github.com/users/jonparrott/following{/other_user}","gists_url":"https://api.github.com/users/jonparrott/gists{/gist_id}","starred_url":"https://api.github.com/users/jonparrott/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonparrott/subscriptions","organizations_url":"https://api.github.com/users/jonparrott/orgs","repos_url":"https://api.github.com/users/jonparrott/repos","events_url":"https://api.github.com/users/jonparrott/events{/privacy}","received_events_url":"https://api.github.com/users/jonparrott/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/jonparrott/goth/labels/Discussion","name":"Discussion","color":"1d76db"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-09-27T20:44:19Z","updated_at":"2016-09-27T20:44:38Z","closed_at":null,"body":"This library needs to satisfy two orthogonal goals related to HTTP:\r\n\r\n1. It needs to be able to make HTTP requests internally (e.g., in [refresh()](https://github.com/jonparrott/goth/blob/master/google/auth/service_account.py#L260))\r\n2. It needs to allow users to attach credentials to an HTTP object of their choosing.\r\n\r\nIf there were only one Python HTTP library this would all be trivial. If the library was called `unicorn`, we could just use `unicorn.request` directly everywhere internally for (1). For (2), we would just write a single function to attach the credentials to `unicorn`.\r\n\r\nThis discussion is exclusively about (1), but care should be made not to limit our options for (2).\r\n\r\n# Use Case (1)\r\n\r\nUse case (1) boils down to the need for a consistent way to make an http request and get response information. This use-case is only for library internals. \r\n\r\n## What's done today\r\n\r\n`transport.request` takes any http object and then uses isinstance to figure out which transport should be used to call. Basically:\r\n\r\n```python\r\n# Calling code\r\ncredentials.refresh(http, ...)\r\n\r\n# credentials.py\r\ndef refresh(http):\r\n    ...\r\n    r = transport.request(http, ...)\r\n    ...\r\n\r\n# transport.py\r\ndef request(http, method, uri, ...):\r\n    transport_module = get_transport_for_http(http)\r\n    return transport_module.request(http, method, uri, ...)\r\n\r\n# urllib3_transport.py\r\ndef request(http, method, uri, ...):\r\n    return http.request(method, uri)\r\n```\r\n\r\nThe indirection has the benefit that we can accept any supported http object at the call sites of methods like `refresh()`. The drawback is the indirection and the cost of the lookup for every request.\r\n\r\n## What we could do\r\n\r\n### Use http.client\r\n\r\nWe could easily use the built-in `http.client` - but urlilb3 and requests have significant benefits, and properly doing ssl with http.client can be error prone. It also leads us to situation where the http client the user is using and the http client we're using can be different.\r\n\r\n### Bind credentials to http\r\n\r\nPresently credentials are completely independent of http. Any methods that need to make requests must accept an ``http`` parameter. This proposes adding ``credentials.http`` that is set to a new **private** interface ``transport._HTTP``. None of the request-making methods need http as an argument anymore.\r\n\r\nThis has the distinct drawback that credentials can now only be associated with a single http object. It also introduces a new interface (and subclasses for every transport).\r\n\r\n### Just pass in transport_impl.request\r\n\r\nInstead of passing in an http object and letting the lookup happen, make the argument to `refresh()` et al be an interface that matches `transport.request` like this:\r\n\r\n```python\r\ndef request(method, uri, ...)\r\n```\r\nSince the caller of refresh *knows* the transport implementation that's currently being used it can just pass in, for example, `functools.partial(urlilb3_transport.request, bare_http)`. The call stack would then look like this:\r\n\r\n```python\r\n# Calling code\r\nbound_request = functools.partial(urllib3_transport.request, self)\r\ncredentials.refresh(bound_request)\r\n\r\n# credentials.py\r\ndef refresh(request):\r\n    ...\r\n    r = request(...)\r\n    ...\r\n\r\n# urllib3_transport.py\r\n# http provided by functools.partial above.\r\ndef request(http, method, uri, ...):\r\n    return http.request(method, uri)\r\n```\r\n\r\nThis has a lot of benefits:\r\n1. The interface for ``request`` can be made public without making any other part of our transport implementation public.\r\n2. No lookup penalty.\r\n3. ``refresh()`` et al will have well defined argument types.\r\n4. Gives implementors of use case (2) the option of passing in something else for ``request``. :)"},"comment":{"url":"https://api.github.com/repos/jonparrott/goth/issues/comments/249992738","html_url":"https://github.com/jonparrott/goth/issues/1#issuecomment-249992738","issue_url":"https://api.github.com/repos/jonparrott/goth/issues/1","id":249992738,"user":{"login":"jonparrott","id":250995,"avatar_url":"https://avatars.githubusercontent.com/u/250995?v=3","gravatar_id":"","url":"https://api.github.com/users/jonparrott","html_url":"https://github.com/jonparrott","followers_url":"https://api.github.com/users/jonparrott/followers","following_url":"https://api.github.com/users/jonparrott/following{/other_user}","gists_url":"https://api.github.com/users/jonparrott/gists{/gist_id}","starred_url":"https://api.github.com/users/jonparrott/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonparrott/subscriptions","organizations_url":"https://api.github.com/users/jonparrott/orgs","repos_url":"https://api.github.com/users/jonparrott/repos","events_url":"https://api.github.com/users/jonparrott/events{/privacy}","received_events_url":"https://api.github.com/users/jonparrott/received_events","type":"User","site_admin":false},"created_at":"2016-09-27T20:44:38Z","updated_at":"2016-09-27T20:44:38Z","body":"@dhermes"}}
{"actor":{"display_login":"jonparrott"}}