authlib
authlib copied to clipboard
operational issues around requests.Sessions though Django integration
Thanks so much for the work on authlib. We came to using it from Auth0's recommendation. Now that we've been operationalizing it, we hit a difficult problem.
This is sort of one bug and sort of 3 bugs. I joined them to give the maintainers flexibility in how they wanted to respond without losing their individual motivations:
- There's no way to set timeouts on requests to
server_metadata_url
andjwks_uri
. There's no way to pass atimeout
parameter to them (preferably one that that a different timeout than the call toDjangoOAuth2App
that caused theload_server_metadata
function be called) nor can the user ofauthlib
add aHTTPAdapter
subcall withsession.mount
that adds default, overridable timeouts because of the class-level factory variables inauthlib
. This has mattered becauserequests.Session
has infinite timeouts by default and we're seeing double digit second request latencies (esp to the metadata requests) from Auth0's APIs. - the
requests.Sessions
are recreated for "meta" requests toserver_metadata_url
andjwks_uri
, and possibly in other codepaths, which means more reconnects than needed in a Python process lifetime. - As hinted at in 1, there's other work that's support by
requests
but not currently supported by the authlib (throughsession.mount
, but I imagine other things). It's not supported because there's no clear way to override the Adapters onOAuth2Session
, especially when the code is coming to it through the Django integration. Currently, it seems like the only option would be to overrideclient_cls
in a subclass ofDjangoOAuth2App
. But, of course, that only works if you know at type definition time all of the info needed to make it and it's not clear what backwards compatibility expectations to put on it since it's pretty deep in the type hierarchy. Hindsight being 20/20, if the factories were not class-level variables that couldn't be overriden fromOAuth#register
and maybe if some equivalent toOAuth2Session
wrapped arequests.Session
instead of being one, this could be handled from the user ofauthlib
's position. As is, it seems like some API expansion needs to be done.
Thanks so much for the work you've done. These kinds of operational improvements would be huge for us. Do you folks have any ideas on how to manage these issues?
To be clear: we're open to providing some patches (balanced against our other tradeoffs), but the design work seems like it needs maintainer thought to be accepted
1. timeout issue
There is a client_kwargs
parameter, but it is not used in .request
method. This need to be fixed. But all requests will share the same client_kwargs
. I'll fix this.
2. meta requests
This is not a big thing, because those meta data will be cached on instance. If you don't recreate the instance every time, it won't send too many requests to meta endpoint.
3. additions
I would like to accept patches for the session.mount
.
Thanks for your feedback.
Hello, @lepture we're having similar issues with timeouts when calling load_server_metadata
, more specifically, we have no way to add a timeout on OAuth2Session.request
in this case. Would you be open to a PR on this?
@Martin-Lar yes, a PR is welcome.