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

Python OAuth client_credentials support #563

Conversation

moderakh
Copy link
Collaborator

@moderakh moderakh commented Aug 13, 2024

The current OAuth implementation in the Python client does not reuse access tokens. Instead, it exchanges the client-id and client-secret for a new access token with every request. This behavior increases the load on the tokenEndpoint and introduces unnecessary latency in the request processing.

This PR updates the OAuth implementation to oauth reuse access tokens, aligning the Python client with the OAuth behavior of the Scala client, as detailed in this PR for the Scala client.

For additional context on the usage of auth_provider and its functionality, please refer to the description in the original PR for the Spark client.

@moderakh moderakh changed the title [MP-2616][MP-2614][MP-2615] oauth client credentials support Python OAuth client_credentials support Aug 13, 2024
# limitations under the License.
#

from abc import ABC, abstractmethod
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's abc and ABC?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a python pattern for making a class abstract in python by importing ABC from the python standard library, we will have access to abstractmethod annotation etc.
see https://docs.python.org/3/library/abc.html

python/delta_sharing/auth.py Outdated Show resolved Hide resolved
json = """
{
"shareCredentialsVersion": 2,
"type": "persistent_oauth2.0",
"type": "oauth_client_credentials",
Copy link
Collaborator

Choose a reason for hiding this comment

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

will changing this causing any break on existing traffic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, this is already discussed with oracle who introduced this and they are fine with the change as there is no traffic persistent_oauth2.0

Copy link
Collaborator

@linzhou-db linzhou-db left a comment

Choose a reason for hiding this comment

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

Thanks! looks good, could you double check the failure in pre-merge tests?

@moderakh moderakh merged commit 1be4eed into delta-io:main Aug 14, 2024
5 checks passed
@moderakh moderakh deleted the moderakh/oauth-client-credentials-support-python-client branch August 14, 2024 16:42
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