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

fix: Better handling when no cached token exists, with more decriptive error #252

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

jstlaurent
Copy link
Contributor

Changelogs

  • Make sure the ensure_active_token methods will identify that there is no token
  • Surface a better error message when it happens, so users can take action to remediate

Checklist:

  • Was this PR discussed in an issue? It is recommended to first discuss a new feature into a GitHub issue before opening a PR.
  • Add tests to cover the fixed bug(s) or the newly introduced feature(s) (if appropriate).
  • Update the API documentation if a new function is added, or an existing one is deleted.
  • Write concise and explanatory changelogs above.
  • If possible, assign one of the following labels to the PR: feature, fix, chore, documentation or test (or ask a maintainer to do it for you).

Adresses #251

This bug fix will better handle cases where the OAuth token is None. This might result from a new user that has not logged in, tokens that were not cached, or cached credentials that have
expired. When the client is used in the loader functions, and one such situation occurs, ane exception will be raised that includes a better, actionable error message for the user.

@jstlaurent jstlaurent requested a review from cwognum as a code owner January 23, 2025 15:40
@jstlaurent jstlaurent added the fix Annotates any PR that fixes bugs label Jan 23, 2025
@jstlaurent jstlaurent self-assigned this Jan 23, 2025
Copy link
Collaborator

@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

Thanks, @jstlaurent !

polaris/loader/load.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mercuryseries mercuryseries left a comment

Choose a reason for hiding this comment

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

Thanks, @jstlaurent!

@jstlaurent
Copy link
Contributor Author

Moved the check into the context manager's _enter_ method, so it's more systematic.

@jstlaurent jstlaurent merged commit 41209c6 into main Jan 23, 2025
19 checks passed
@jstlaurent jstlaurent deleted the fix/external-auth-token branch January 23, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Annotates any PR that fixes bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants