Skip to content

Fix asyncify for users client where token is not required - #506

Merged
adamjmcgrath merged 4 commits into
auth0:masterfrom
cgearing:master
Jul 24, 2023
Merged

adamjmcgrath merged 4 commits into
auth0:masterfrom
cgearing:master

Conversation

@cgearing

@cgearing cgearing commented Jul 10, 2023 •

Copy link
Copy Markdown
Contributor

Changes

This PR fixes a bug that is currently in the Auth0 SDK asyncify wrapper where it falls through to the AsyncManagementClient when the class being wrapped doesn't inherit from the AuthenticationBase class. This doesn't work for the Users client, as that shouldn't require a management token.

Currently, using the Users client with the async wrapper fails with the following error:

TypeError: asyncify.<locals>.AsyncManagementClient.__init__() missing 1 required positional argument: 'token'

The approach I took was to implement a new BareAsyncClient for this case, however I'm not 100% wedded to this approach so feel free to tell me to have another go!

Testing

I've added a test for this functionality to check that we don't need to add the token when calling the Users client when wrapped in an async context.

  • This change adds integration test coverage

Checklist

@cgearing
cgearing requested a review from a team as a code owner July 10, 2023 09:44
Comment thread auth0/asyncify.py Outdated
if callable(getattr(cls, func)) and not func.startswith("_")
]

class BareAsyncClient(cls):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this fix @cgearing! Would you mind just calling this UsersAsyncClient and I'd be happy to approve the PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! Sorry I missed your comment!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @cgearing!

@cgearing
cgearing requested a review from adamjmcgrath July 24, 2023 13:56
@adamjmcgrath
adamjmcgrath merged commit c5131b6 into auth0:master Jul 24, 2023
@adamjmcgrath adamjmcgrath mentioned this pull request Jul 25, 2023
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