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

Documented PyJWT dependency and improved logging and exception messages #458

Merged
merged 7 commits into from
Aug 2, 2017
Merged

Conversation

hoylen
Copy link
Contributor

@hoylen hoylen commented Jan 20, 2017

Updated documentation so it is clear that PyJWT (and not any jwt library) is needed. Corrected "pip install" command to install "pyjwt" instead of "jwt".

Added more information to the NotImplemented exception messages. Instead of saying "Subclasses must implement this function", it is now more useful because it says "Missing function implementation in className: nameOfMissingFunction".

Extra logging has been added to help in debugging. During development, the signature base string signed by the client may be different from the one independently calculated by the server: logging these string helps debug such problems.

In oauthlib/oauth1/rfc5849/endpoints/signature_only.py exceptions are caught and then False is simply returned. The details in those exceptions are useful for debugging what went wrong. Their values are now logged, rather than simply discarded.

@@ -2,7 +2,7 @@
Creating a Provider
===================

OAuthLib is a dependency free library that may be used with any web
OAuthLib is a framework independent library that may be used with any web
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because OAuthLib has dependencies, so it is not "dependency free". The description of "dependency free" would mean it does not need any other package to run. OAuthLib (as indicated in the requirements.txt file) needs PyJWT, blinker and cryptography packages to function.

I'm trying to guess what the original author really meant, and assume they meant to say it can be used with any framework (e.g. doesn't have to be Django) rather than "dependency free" (which it is clearly not, since it has dependencies).

@thedrow
Copy link
Collaborator

thedrow commented Jul 31, 2017

Can you please rebase this branch?

@thedrow
Copy link
Collaborator

thedrow commented Aug 1, 2017

Looks like the rebase went really wrong here. Can you try again?

@hoylen
Copy link
Contributor Author

hoylen commented Aug 1, 2017

Are you referring to the red cross next to 13387a4? There was a bug in my code, so it was failing the automatic checks. That was then fixed in b3f4cb9.

@thedrow
Copy link
Collaborator

thedrow commented Aug 1, 2017

I'm referring to the entire code history in this PR that isn't related. See #458 (commits)

@hoylen
Copy link
Contributor Author

hoylen commented Aug 1, 2017

Done. This should be better.

@thedrow thedrow merged commit 612ac5b into oauthlib:master Aug 2, 2017
@thedrow
Copy link
Collaborator

thedrow commented Aug 2, 2017

Thanks!

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.

None yet

2 participants