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

RESTClient: implement AuthConfigBase.__bool__ + update docs #1413

Merged
merged 4 commits into from
May 27, 2024

Conversation

rudolfix
Copy link
Collaborator

@rudolfix rudolfix commented May 25, 2024

Description

This PR is resubmitted because I merged the last one with typing problems that made linting to fail. Let's try to fix our qdrant problem so we see CI green again...

linting fixes included

  • using correct types in Auths defined in tests
  • changing the types for auth in RESTClient to AuthBase which is a common base for AuthConfigBase and the one used by requests (AuthBase itself)

all tests including those in rest api source are passing after this


This PR implements bool on AuthConfigBase so instances of this class evaluate to True in requests.sessions.Session.prepare_request() when auth argument is evaluated otherwise instances of AuthConfigBase have no effect when passed as arguments to RESTClient's methods.

The PR also updates the documentation to suggest explicit inheritance from requests.auth.AuthBase for custom authentication.

Copy link

netlify bot commented May 25, 2024

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit 046308f
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6651af255e77840008e61022

@rudolfix rudolfix requested a review from burnash May 25, 2024 09:30
Copy link
Collaborator

@burnash burnash left a comment

Choose a reason for hiding this comment

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

LGTM

@rudolfix rudolfix merged commit 4fcfa28 into devel May 27, 2024
4 checks passed
@rudolfix rudolfix deleted the fix/restclient_bool_authconfigbase branch May 27, 2024 13:04
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.

2 participants