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

feat: Support requests.auth authenticators #1109

Merged
merged 9 commits into from
Feb 3, 2023
Merged

Conversation

edgarrmondragon
Copy link
Collaborator

@edgarrmondragon edgarrmondragon commented Oct 25, 2022

Closes #1105


📚 Documentation preview 📚: https://meltano-sdk--1109.org.readthedocs.build/en/1109/

@edgarrmondragon edgarrmondragon force-pushed the feat/requests-auth branch 2 times, most recently from 115c993 to 869b0b2 Compare October 25, 2022 23:50
@edgarrmondragon edgarrmondragon force-pushed the feat/requests-auth branch 2 times, most recently from e42d47d to b4dc4fc Compare December 6, 2022 23:53
@edgarrmondragon edgarrmondragon marked this pull request as ready for review December 7, 2022 00:03
@codecov
Copy link

codecov bot commented Dec 7, 2022

Codecov Report

Merging #1109 (c6caacc) into main (7ec2a71) will increase coverage by 0.06%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##             main    #1109      +/-   ##
==========================================
+ Coverage   85.12%   85.19%   +0.06%     
==========================================
  Files          54       54              
  Lines        4694     4708      +14     
  Branches      798      800       +2     
==========================================
+ Hits         3996     4011      +15     
+ Misses        507      506       -1     
  Partials      191      191              
Impacted Files Coverage Δ
singer_sdk/authenticators.py 65.76% <92.30%> (+1.59%) ⬆️
singer_sdk/streams/rest.py 86.41% <100.00%> (+1.51%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@aaronsteers
Copy link
Contributor

aaronsteers commented Feb 3, 2023

@edgarrmondragon - This one went under my radar and I'm just reading through now. Can you add a usage example in one of the sample implementations, or in the Code Samples section, or similar? I'm not super familiar with this and I think a sample usage would be helpful also for devs who want to leverage this.

Or is it proper to think of this as fully internal to the SDK and the dev-side experience is basically the same?

@edgarrmondragon
Copy link
Collaborator Author

@edgarrmondragon - This one went under my radar and I'm just reading through now. Can you add a usage example in one of the sample implementations, or in the Code Samples section, or similar? I'm not super familiar with this and I think a sample usage would be helpful also for devs who want to leverage this.

Or is it proper to think of this as fully internal to the SDK and the dev-side experience is basically the same?

@aaronsteers thanks for taking a look! I've added a simple example with links to community-supported authentication classes:

https://meltano-sdk--1109.org.readthedocs.build/en/1109/code_samples.html#use-one-of-requests-s-built-in-authenticators

@aaronsteers
Copy link
Contributor

@edgarrmondragon - Super helpful!! Thank you!

I was going to ask for links and then realized they are already linked 😅.

LGTM!

Copy link
Contributor

@aaronsteers aaronsteers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge when ready. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Allow built-in request authentication helpers to be used
4 participants