Skip to content
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

[Feature]: Allow built-in request authentication helpers to be used #1105

Closed
edgarrmondragon opened this issue Oct 24, 2022 · 0 comments · Fixed by #1109
Closed

[Feature]: Allow built-in request authentication helpers to be used #1105

edgarrmondragon opened this issue Oct 24, 2022 · 0 comments · Fixed by #1109
Assignees
Labels
kind/Feature New feature or request valuestream/SDK

Comments

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Oct 24, 2022

Feature scope

Taps (catalog, state, stream maps, etc.)

Description

Some of these are nice, and would at least remove from duplication on our side:

https://requests.readthedocs.io/en/latest/api/#authentication

Custom authenticators are also interesting, and developers would be able to leverage existing auth packages in the wild:

https://requests.readthedocs.io/en/latest/user/advanced/#custom-authentication


The implementation seems easy enough as we already did most of the work in #842. We'd only need to add a __call__ method to the base auth class:

class APIAuthenticatorBase:
    def __call__(self, request):
        return self.authenticate_request(request)

and also set the auth attribute of the session:

class RESTStream:
    def build_prepared_request(
        self,
        *args: Any,
        **kwargs: Any,
    ) -> requests.PreparedRequest:
        ...
        self.requests_session.auth = self.authenticator
        return self.requests_session.prepare_request(request)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/Feature New feature or request valuestream/SDK
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant