Skip to content
This repository was archived by the owner on Jun 23, 2023. It is now read-only.

Conversation

nsklikas
Copy link
Contributor

The expires_in defined in usage rules was not reflected to the access token when it was a JWT

@nsklikas nsklikas requested review from peppelinux and rohe August 16, 2021 10:34
@nsklikas nsklikas changed the title Fix bug Fix JWT access token lifetime Aug 16, 2021
self,
session_id: Optional[str] = "",
ttype: Optional[str] = "",
usage_rules: Optional[dict] = None,
Copy link
Member

Choose a reason for hiding this comment

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

None or {}?

it's a dict, probably {} Is better

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've always (well almost) refrained from using mutable values as defaults. So I'd vote for None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, a dict for a default value is a bug waiting to happen

Copy link
Member

Choose a reason for hiding this comment

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

I've always thought about why a variable of one type could take on another type, such as None. But it's a completely cosmetic thing in python, let's move on

@peppelinux
Copy link
Member

@nsklikas we just have to patch the conflicting files, then merge

@nsklikas nsklikas force-pushed the feature-jwt-access-lifetime branch from 2fc90e6 to 4c4e1e0 Compare August 23, 2021 08:52
@peppelinux peppelinux force-pushed the develop branch 2 times, most recently from 406068c to 78cae68 Compare September 2, 2021 23:45
@peppelinux peppelinux merged commit 7bbde28 into IdentityPython:develop Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants