-
Notifications
You must be signed in to change notification settings - Fork 32
♻️ Refactor and Upgrade Users Repository including users_secrets split 🗃️
#8124
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
♻️ Refactor and Upgrade Users Repository including users_secrets split 🗃️
#8124
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8124 +/- ##
==========================================
- Coverage 88.22% 88.11% -0.11%
==========================================
Files 1890 1889 -1
Lines 72631 72677 +46
Branches 1277 1274 -3
==========================================
- Hits 64076 64038 -38
- Misses 8174 8262 +88
+ Partials 381 377 -4
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
dc7c7bb to
f217f7e
Compare
d0620b8 to
649ab2d
Compare
users_secrets split
…espan and update deprecation note in insert_user_and_secrets
…s regardless of product name
d0a0938 to
177de8c
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.
Pull Request Overview
This PR refactors the users repository by extracting user passwords into a new users_secrets table and upgrades the UsersRepo to centralize user management logic. The refactoring includes removing deprecated aiopg dependencies and consolidating user-related fixtures into a centralized helper module.
- Introduces new
users_secretstable with database migration to move password hashes fromuserstable - Extends
UsersRepoclass to use modern async SQLAlchemy patterns and removes legacy aiopg code - Consolidates test fixtures using new
pytest_simcore.helpers.postgres_usersmodule
Reviewed Changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/postgres-database/src/simcore_postgres_database/models/users_secrets.py | New table model for storing user password hashes separately |
| packages/postgres-database/src/simcore_postgres_database/utils_users.py | Major refactor to use async SQLAlchemy and new table structure |
| packages/pytest-simcore/src/pytest_simcore/helpers/postgres_users.py | New helper module for creating user+secrets test fixtures |
| services/web/server/src/simcore_service_webserver/login/_auth_service.py | Updated authentication service to work with new user repository structure |
| Multiple test files | Updates to use new consolidated user fixture patterns |
services/web/server/src/simcore_service_webserver/login/_login_service.py
Outdated
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/login/_auth_service.py
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/login/_controller/rest/twofa.py
Show resolved
Hide resolved
GitHK
left a comment
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.
thanks
services/web/server/src/simcore_service_webserver/login/_controller/rest/confirmation.py
Show resolved
Hide resolved
sanderegg
left a comment
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.
Thanks a bunch. I'm going to not like the conflicts but that is good.
I don't understand why you kept all these fixtures that do the same as create_registered_user. This comes in a subsequent PR?
packages/postgres-database/src/simcore_postgres_database/models/users_secrets.py
Show resolved
Hide resolved
services/web/server/src/simcore_service_webserver/login/_controller/rest/change.py
Outdated
Show resolved
Hide resolved
matusdrobuliak66
left a comment
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.
Thanks!
|
users_secrets splitusers_secrets split 🗃️



What do these changes do?
This PR refactors and upgrades the users repository, impacting the
users,users_pre_registration, and introducing the newusers_secretstable.users_secretstable (Rename tables to include_secretif they include secrets that cannot be monitored #8060)users-related fixtures usingpytest_simcore.helpers.postgres_userstest_users_secrets_migration_upgrade_downgradeUsersRepoto centralize user management logicaiopgdependencies fromUsersRepotests (migrate code from aiopg to asyncpg #4529)_login_repository_legacy.AsyncpgStorage(Maintenance: webserver-login refactoring #7344)Highlights on
pytest_simcore.helpers.postgres_userspytest_simcore.helpser.postgres_tools.insert_and_get_row_lifespanimplements a test data factory pattern for creating database fixtures with proper relationship handling. Now we specialized to create users in the database:random_user,random_user_secrets) to generate realistic test data for bothusersandusers_secretstablesuser_id=user["id"]AsyncExitStackto properly clean up both database records when the context exitsThe function follows the insert-and-get pattern using
insert_and_get_row_lifespanfor each table, which:This is commonly used in pytest fixtures to create temporary, isolated test data for users that gets automatically cleaned up after each test.
Related issue/s
_secretif they include secrets that cannot be monitored #8060How to test
Dev-ops
None