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

Added function to retry login with refresh JWT token #69

Merged
merged 2 commits into from Sep 10, 2019

Conversation

@prashant-shahi
Copy link
Member

commented Sep 9, 2019

This resolves issue #56

  • Added retryLogin function to refresh JWT token
  • Added isJwtExpired and isUnauthenticatedError functions to detect if JWT token has expired or unauthenticated

This change is Reviewable

@prashant-shahi prashant-shahi requested review from paulftw and danielmai Sep 9, 2019

@pullrequest
Copy link

left a comment

A review job has been created and sent to the PullRequest network.


Check the status or cancel PullRequest code review here.

@prashant-shahi prashant-shahi changed the title Added function to retry login with refresh JWT token #56 Added function to retry login with refresh JWT token Sep 9, 2019

@coveralls

This comment has been minimized.

Copy link

commented Sep 9, 2019

Coverage Status

Coverage decreased (-4.0%) to 89.286% when pulling cf4ff9e on prashant/refresh-token into 62ae0fe on master.

@pullrequest
Copy link

left a comment

👍 this looks good to me. Some of the code seemed like it might be autogenerated so I held off commenting on its style. If it is not autogenerate then I would have more comments.


Reviewed with ❤️ by PullRequest

src/client.ts Show resolved Hide resolved
lib/clientStub.js Show resolved Hide resolved
@danielmai
Copy link
Member

left a comment

:lgtm:. The refresh JWT change here works with a cluster set up with a short ACL TTL (--acl_access_ttl 3s --acl_cache_ttl 5s).

Reviewable status: 0 of 14 files reviewed, 2 unresolved discussions (waiting on @danielmai, @paulftw, and @pullrequest[bot])

@pullrequest
Copy link

left a comment

Few ideas/suggestions that would make likely make future reviews/changes easier, but otherwise this code looks straightforward and well-written.


Reviewed with ❤️ by PullRequest

lib/client.js Show resolved Hide resolved
src/txn.ts Show resolved Hide resolved
src/client.ts Outdated Show resolved Hide resolved
src/txn.ts Show resolved Hide resolved
lib/util.js Show resolved Hide resolved

@prashant-shahi prashant-shahi merged commit 9361228 into master Sep 10, 2019

4 of 5 checks passed

code-review/reviewable 14 files, 7 discussions left (paulftw, pullrequest[bot])
Details
Build (dgraph-js) TeamCity build finished
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@prashant-shahi prashant-shahi deleted the prashant/refresh-token branch Sep 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.