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

New Feature: Add Token #11

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

New Feature: Add Token #11

wants to merge 12 commits into from

Conversation

slarrain
Copy link

This Pull Request adds a new functionality to the pydkan package: the ability to not only use username/password but also tokens.

The Good

  • There is no difference in the way we use this package at all. Only at the beginning, when creating the instance, instead of using api = DatasetAPI(uri, user, password) we do api = DatasetAPI(uri, token) assuming token has already being defined.
  • There were very small changes in some functions to allow this to work, but not on the "user space". The functionality is the same.
  • The examples has been updated to use tokens.
  • The README has been updated.

The Bad

  • There is one thing that could break this implementation. Doing api = DatasetAPI(uri, token, True). If using token and want to set debug to True the way to do that is api = DatasetAPI(uri, token, debug=True).
  • There is no token argument (at least with that name), so api = DatasetAPI(uri, token=token, debug=True) won't work.

The Missing

  • I haven't updated the tests. If this PR gets accepted, I will definitely do that (and make another PR).

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