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

Changing flask-security to flask-security-too #162

Merged
merged 8 commits into from
Dec 10, 2022

Conversation

tuz666
Copy link
Collaborator

@tuz666 tuz666 commented Nov 28, 2022

I opened this new branch because the previous got messed up during a rebase.

The currently used flask-security package is no longer supported (latest change is 2,5 years old), flask-security-too is the maintained up-to-date version of it.

Previously we talked about @gammaw that introducing the authentication made the frontend page is a bit slower.
This new security package offers some performance boost with version 3.1.0 and 3.2.0, for these uplifts we need a modification which looks like a DB change but it's not that. I checked the use of lazy parameter according to this article and it looks like we don't use it directly anywhere in the code, so no other changes needed.

We agreed that we should do some performance measurement before these upgrades:

  • @gammaw suggested to do a profiling on the front end page to see exactly how much time each transaction takes
  • I recommended to extend our CI with a robustness/benchmark test which can simulate a big load of incoming GET requests and measure the time it takes.

I extended the CI tests with the robustness/benchmark part, you can see the results for flask-security-too version 3.0.2 here and 3.2.0 here, it wasn't a big performance break-through :(

@tuz666 tuz666 temporarily deployed to dev November 28, 2022 16:17 Inactive
@tuz666 tuz666 temporarily deployed to dev November 28, 2022 18:32 Inactive
@tuz666 tuz666 marked this pull request as ready for review December 4, 2022 14:09
@tuz666 tuz666 requested review from pvj, baxgas, gammaw and molnar-a December 4, 2022 14:09
Copy link
Member

@gammaw gammaw left a comment

Choose a reason for hiding this comment

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

Go, go! Big up for the robustness tests addition!

@tuz666 tuz666 merged commit 260d97a into master Dec 10, 2022
@pvj pvj deleted the dev/use-flask-security-too-instead-of-flask-security branch January 29, 2023 13:43
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.

2 participants