-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add module for generating JWTs #2948
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2948 +/- ##
==========================================
+ Coverage 60.47% 60.48% +0.01%
==========================================
Files 603 604 +1
Lines 43771 43792 +21
Branches 48 48
==========================================
+ Hits 26469 26489 +20
- Misses 17290 17291 +1
Partials 12 12 ☔ View full report in Codecov by Sentry. |
524896f
to
49ee048
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me. Why is 'name' and 'aud' identical?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks a-ok, but I have questions I want clarified before I give the green light :)
yield "nav" | ||
|
||
|
||
@pytest.fixture(scope="module") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious why you picked "module" scope for this fixture?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i remember correctly, fixtures with "module" scope can only use other fixtures that are also "module" scope or higher, so since jwtconf_mock
(which is "module" scope) uses this, this also has to be "module" scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I ask is that I can't recall ever seeing any particular use-case for module
scoped fixtures.
I.e. test
scoped fixtures are run on every test. module
scoped fixtures are only run once per test module. session
-scoped fixtures are run once per full test session.
If a test fixture is only ever defined and used within a single module, then I don't see that there is a principal difference between assigning module
or session
scope to it, so I was curious why you picked module
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see what you mean. I think my rationality here was to use the lowest level scope that still fulfilled the needs. I looked at the options and module
seemed to fit the best in this case.
yield "nav" | ||
|
||
|
||
@pytest.fixture(scope="module") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I ask is that I can't recall ever seeing any particular use-case for module
scoped fixtures.
I.e. test
scoped fixtures are run on every test. module
scoped fixtures are only run once per test module. session
-scoped fixtures are run once per full test session.
If a test fixture is only ever defined and used within a single module, then I don't see that there is a principal difference between assigning module
or session
scope to it, so I was curious why you picked module
.
054974b
to
cd9e8fb
Compare
#2481
partial replacement for #2569
Adds functions for generating access and refresh Tokens of the Jason Web variety.
The expiry values should probably be configurable, but I'd rather do that in a separate PR since it would require
deciding the granularity of expiry (seconds, minutes, hours), updating docs etc etc ..