-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve docs around adding authorization header mechanism able to refresh tokens #552
Comments
Do you have thoughts on how this would compare to something like the azure SDK's https://learn.microsoft.com/en-us/python/api/overview/azure/identity-readme?view=azure-python has some background. |
It's a similar pattern. I think we could get away with a callable here, and then have auth implementation define a class that serves as a provider, either giving by giving users examples to copy in or publishing something that you can easily pass to a Client/passes back a configured Client. |
I'm not opposed to the idea outright, but I am getting a little bit of a twitch about adding another "tweak behavior" attribute to As an alternative, I wonder if there's some refactoring we can do of class PlanetaryComputerStacIO(StacApiIO):
def update_headers(self, headers: Dict[str, Any]):
# This overrides a no-op superclass method
...
client = Client.open("https://planetarycomputer.microsoft.com/api/stac/v1", stac_io=PlanetaryComputerStacIO()) |
I'll keep linking to https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/core/azure-core/CLIENT_LIBRARY_DEVELOPER.md on this topic :) I think you really do need that "chained policy" concept for these things to compose well together and we can maybe get some code-reuse for the different authorization mechanisms. As for this feature request: since we're already exposing so many of the details of the I/O currently that it's not much worse to add one more. |
I actually didn't catch from requests import Request
def req_modifier(req: Request) -> Request:
req.headers["Authorization"] = f"Bearer {tp.get_token().token}"
return req
client = pystac_client.Client.open("https://foo.westeurope.cloudapp.azure.com/stac", request_modifier=req_modifier) Works with the above definition of I'm fine with leveraging this feature rather than adding a new Unless we think a more top-level |
Currently the way to add an Authorization header to a Client is to add a static token in the
headers
parameter. This works, however if the token can expire, the user has to stay on top of checking token expiration any time they want to use the client and callclient._stac_io.update(headers=...)
.I propose we add an
authorization
parameter toClient
andStacApiIO
that can either take a string or a function. This parameter would provide theAuthorization:
header for each request. If it's a string, just use that string - this would be the same as supplying it in theheaders
parameter, but would take precedence over anyAuthorization
key in the headers. If the value were a function, it would call that function to get the value of theAuthorization
header for each request made byStacApiIO
. That way, I could write a function that checks the expiration of the token and uses a new one in case it's about to expire. E.g.:The text was updated successfully, but these errors were encountered: