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

[backend-comparison] Refresh access token and display authenticated user name #1483

Merged
merged 7 commits into from
Mar 20, 2024

Conversation

syl20bnr
Copy link
Member

Pull Request Template

Checklist

  • Confirmed that run-checks all script has been executed.
  • Made sure the book is up to date with changes in this PR.

Related Issues/PRs

#1072

Changes

Add automatic refresh of access token. Now both the access token and the refresh token are cached. The refresh is done by the user benchmark server.

A new endpoint /users/me in the user benchmark server returns the anonymized user name so that we can display it when executing burnbench auth.

The auth command is now idempotent and can also refresh the access token so the CLI should now ask for the user's authorization only when strictly required.

Testing

Tested with a local stack.

Copy link

codecov bot commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 14.10256% with 134 lines in your changes are missing coverage. Please review.

Project coverage is 85.76%. Comparing base (4e68cb2) to head (5db22b0).
Report is 10 commits behind head on main.

Files Patch % Lines
backend-comparison/src/burnbenchapp/auth.rs 19.29% 92 Missing ⚠️
backend-comparison/src/persistence/base.rs 0.00% 27 Missing ⚠️
backend-comparison/src/burnbenchapp/base.rs 0.00% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1483      +/-   ##
==========================================
+ Coverage   85.75%   85.76%   +0.01%     
==========================================
  Files         647      655       +8     
  Lines       72300    73251     +951     
==========================================
+ Hits        61998    62823     +825     
- Misses      10302    10428     +126     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@syl20bnr syl20bnr force-pushed the feat/refresh-access-token-display-nickname branch 2 times, most recently from 894b247 to 29c82ae Compare March 18, 2024 16:54
Copy link
Collaborator

@antimora antimora left a comment

Choose a reason for hiding this comment

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

LGTM

@syl20bnr syl20bnr force-pushed the feat/refresh-access-token-display-nickname branch 2 times, most recently from 0156daf to c3b37e0 Compare March 19, 2024 02:03
Copy link
Member

@nathanielsimard nathanielsimard left a comment

Choose a reason for hiding this comment

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

Minor changes

backend-comparison/README.md Outdated Show resolved Hide resolved
backend-comparison/src/burnbenchapp/auth.rs Outdated Show resolved Hide resolved
backend-comparison/src/burnbenchapp/mod.rs Show resolved Hide resolved
@syl20bnr syl20bnr force-pushed the feat/refresh-access-token-display-nickname branch from 2b9240b to c9a9def Compare March 19, 2024 19:59
@syl20bnr
Copy link
Member Author

@nathanielsimard ready for another round.

@syl20bnr syl20bnr force-pushed the feat/refresh-access-token-display-nickname branch 2 times, most recently from 3dbfc8b to 5db22b0 Compare March 19, 2024 21:41
The reqwest must have an explicit empty body otherwise the release
build returns a 411 when refreshing the tokens without even calling
the benchmark server endpoint.
@syl20bnr syl20bnr merged commit e8863da into main Mar 20, 2024
14 of 15 checks passed
@syl20bnr syl20bnr deleted the feat/refresh-access-token-display-nickname branch March 20, 2024 19:39
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.

3 participants