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.

Sign up for GitHub

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

Jump to bottom

Add the user's sub to the information returned by /user/limits #188

Merged
merged 2 commits into from
Apr 6, 2022
Merged

Add the user's sub to the information returned by /user/limits #188

merged 2 commits into from
Apr 6, 2022

Conversation

ro-tex
Copy link
Contributor

@ro-tex ro-tex commented Apr 5, 2022

PULL REQUEST

Overview

Add the user's sub to the information returned by /user/limits, so that can be cached and cross-referenced later in nginx's access logs.

Example for Visual Changes

Checklist

Review and complete the checklist to ensure that the PR is complete before assigned to an approver.

  • All new methods or updated methods have clear docstrings
  • Testing added or updated for new methods
  • Verify if any changes impact the WebPortal Health Checks
  • Approriate documentation updated
  • Changelog file created

Issues Closed

Closes #181

@ro-tex ro-tex self-assigned this Apr 5, 2022
peterjan
peterjan approved these changes Apr 6, 2022
View reviewed changes
Copy link

@peterjan peterjan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, I do have one question though, now that the cache holds both the tier and the quota exceeded, did we also ensure the cache is properly invalidated not only when the tier changes but when the user exceeds his quota?

@peterjan peterjan merged commit d7c68f8 into main Apr 6, 2022
@peterjan peterjan deleted the ivo/limits_sub branch April 6, 2022 09:08
@ro-tex
Copy link
Contributor Author

ro-tex commented Apr 6, 2022

LGTM, I do have one question though, now that the cache holds both the tier and the quota exceeded, did we also ensure the cache is properly invalidated not only when the tier changes but when the user exceeds his quota?

Yes, that's covered by TestUserTierCache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwypchlo kwypchlo kwypchlo approved these changes

@peterjan peterjan peterjan approved these changes

@ChrisSchinnerl ChrisSchinnerl Awaiting requested review from ChrisSchinnerl ChrisSchinnerl is a code owner

Assignees

@ro-tex ro-tex

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose the user's sub on /user/limits
3 participants
@ro-tex @peterjan @kwypchlo