-
Notifications
You must be signed in to change notification settings - Fork 32
♻️ Major Refactor: Isolate webserver's user Subdomains & Modernize Internal Structure (🚨)
#8083
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
♻️ Major Refactor: Isolate webserver's user Subdomains & Modernize Internal Structure (🚨)
#8083
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8083 +/- ##
==========================================
+ Coverage 87.60% 88.31% +0.70%
==========================================
Files 1866 1663 -203
Lines 71919 66997 -4922
Branches 1268 938 -330
==========================================
- Hits 63008 59169 -3839
+ Misses 8543 7553 -990
+ Partials 368 275 -93
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
webserver's user Subdomains & Modernize Internal Structure (🚨)
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 domain by isolating its subdomains (user_preferences, user_notifications, user_tokens) into independent modules, updating import paths throughout the codebase and tests, and modernizing bootstrap registration.
- Updated import paths in unit tests to reference new modules (
users_service,user_preferences,user_notifications) instead of legacyusers.apior_common. - Replaced direct calls to internal user APIs with public service facades (
users_service,user_preferences_service,UserNotificationsRepository). - Updated
users/plugin.pyto compose subdomain features via dedicated bootstrappers, and removed legacy REST exception handlers in favor of consolidated_rest_exceptions.
Reviewed Changes
Copilot reviewed 91 out of 106 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| services/web/server/tests/** | Adjusted test imports to the new module structure |
| services/web/server/src//users/ | Replaced users.api imports with users_service or new models |
| services/web/server/src/**/user_* | Introduced user_preferences, user_notifications, user_tokens modules with clear bootstrap and service layers |
| packages/service-library/** | Added ensure_single_setup decorator to observer registry |
Comments suppressed due to low confidence (1)
packages/service-library/src/servicelib/aiohttp/observer.py:24
- The variable 'log' is not defined in this module, leading to a NameError at runtime. It should reference a valid logger (e.g., import or define
_logger) or use a correct logger variable.
@ensure_single_setup(__name__, logger=log)
odeimaiz
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.
👀
Let me know when you want check the notifications 👍
services/web/server/src/simcore_service_webserver/garbage_collector/_core_orphans.py
Show resolved
Hide resolved
|
bisgaard-itis
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 for the effort

What do these changes do?
This PR introduces a major refactoring of the
usersdomain in thewebserver, one of the oldest and most coupled parts of the codebase. Please read this description carefully.The primary goal is to decouple user-related subdomains — such as
user_preferences,user_tokens, anduser_notifications— into self-contained modules. Although conceptually tied to the user, each of these features has its own database table and business logic, and they are now implemented as independent domains.As a result, the
usersdomain now focuses solely on user management (i.e., logic around theuserstable). It continues to serve as the main entry point, with itsbootstrap.pyfunction still responsible for composing and initializing all related subdomains viasetup_users().In addition, this PR includes:
user_preferencesanduser_notificationsaiopgdependencies (migrate code from aiopg to asyncpg #4529)The changes are backed by comprehensive test coverage to ensure stability.
Further details include an overview of all
usersdomain modules, a breakdown of the new structure usinguser_preferences/as an example, and notes on theuser_notificationsrefactor.Skeleton and CSR-layer layout:
Here an explanation of the structure using as example
user_preferences/:bootstrap.py(public)Initializes the domain into the aiohttp app: implements
setup_*(app)functionon_startup/on_cleanup-like eventsappstate_controller/rest/*_rest.pyREST handler module
_controller/rest/_rest_exceptions.pyREST-specific exceptions handlers:
_models.pyDomain model definitions (likely ORM or DTOs):
_repository.pyData access layer:
_service.pyDomain logic layer:
user_preferences_service.py(public)Public service interface:
user_preferencesfrom outside (e.g., fromusers/bootstrap.py)Refactoring of
user_notificationsuser_notifications._repository.py): handles: Redis commands, data serialization/deserialization, key management. Why?1. Data Access Pattern: The Redis operations are pure data access/persistence operations - they're interacting with the data store (Redis) just like database operations would interact with PostgreSQL.
2. Consistency with existing pattern: Looking at your other modules:
-
user_preferenceshas a repository that handles database operations viaFrontendUserPreferencesRepo-
user_tokenshas a repository that handles database operations via SQLAlchemy- The Redis operations here are functionally equivalent - they're data persistence operations
3. Separation of Concerns
4. Testability: Having Redis operations in a repository makes them easier to mock and test
user_notifications._service.py): Handle business logic (product filtering, backwards compatibility, notification creation logic), call repository methodsuse_notification_rest.py) : Handle HTTP concerns, validation, call service methodsRelated issue/s
login_accounts#8080usersdomain inwebserver#6913How to test
Manual exploratory testing 🚨
Dev-ops
None