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

Refine activation code generation. #1225

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

weakish
Copy link

@weakish weakish commented Aug 10, 2022

Previously, activation code genaration invoked a redudant str() conversion
(secrets.cohice(string.digits) already returns a string).
Also, secrets.choice was called six times, which is slower than just one call.
I hope this refinement make the code more readable and faster.

Previously, activation code genaration invoked a redudant `str()` conversion
(`secrets.cohice(string.digits)` already returns a string).
Also, secrets.choice was called six times, which is slower than just one call.
I hope this refinement make the code more readable and faster.
@acasajus
Copy link
Collaborator

Hey, thanks for contributing! May I suggest a small improvement.

At the top define

ACTIVATION_CODE_NUM_DIGITS=6

and then we can do

str(secrets.randbelow(10**ACTIVATION_CODE_NUM_DIGITS)).zfill(ACTIVATION_CODE_NUM_DIGITS)

👍

@acasajus acasajus added the enhancement New feature or request label Aug 10, 2022
@weakish
Copy link
Author

weakish commented Aug 10, 2022

I applied suggestion in e2b147f (I defined the const in config.py instead of the top of auth.py since it seems most constants are defined there.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants