Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 28, 2025

What do these changes do?

This PR is a follow up from #7263 where we increased coveraged and restructured the product domain in the webserver. This PR does a number of a upgrades of the functionality and small changes in the structure

Changes in simcore-postgres-database

  • utils_products*.py migrated from aiopg to asyncpg
  • renamed errors -> aiopg_errors because those are not raised by sqlalchemy.exc.asyncio functionality

Changes in web-server's productdomain

  • Moves controllers to a folder
products/
├── _controller/              # 🔒 Internal: many controllers Layer (REST/RPC)
│   ├── __init__.py           
│   ├── rest.py                        # 🔒 Internal: REST API handlers
│   ├── rest_exceptions.py   # 🔒 Internal: REST API exception handlers
│   ├── rest_schemas.py      # 🔒 Internal: REST API schemas (Pydantic)
│   ├── rpc.py                # 🔒 Internal: RPC Controller
│
├── __init__.py               
├── errors.py                 # ✅ Public: Domain-specific errors
├── models.py                 # ✅ Public: Domain models (DTOs, Pydantic)
├── plugin.py                 # ✅ Public: Domain setup within the `web.Application`
├── products_service.py      # ✅ Public: The main service interface (used by other domains)
├── _repository.py            # 🔒 Internal: Data access layer (SQLAlchemy, etc.)
├── _service.py               # 🔒 Internal: Internal service logic (not exposed outside `products`)
├── _web_events.py            # 🔒 Internal: Web event listeners (startup/shutdown, etc.)
├── _web_helpers.py           # 🔒 Internal: Web-related utilities
├── _web_middlewares.py       # 🔒 Internal: Middleware functions (detects products, logging, etc.)
  • upgrades to asyncpg
    • new repository base BaseRepository
    • upgrades ProductRepository
    • replace aiopg from many test fixtures
  • Extended domain errors
  • New REST exception handlers
  • Decoupled schemas from domain models: CreditResult, ProductStripIfon and PaymentFields

Changes in web-server's api-keysdomain

@giancarloromeo @bisgaard-itis please carefully review these changes

  • Resolved Test Flakiness: The test's instability was due to the garbage collector (GC) not being disabled (it was introduced with test_prune_expired_api_keys_task_is_triggered ) while also calling to delete explicitly. The solution was creating a new fixture disabled_setup_garbage_collector to disabling GC
  • Corrected Imports and Annotations: Adjusted import statements and annotations to ensure proper functionality and code clarity.
  • Fixed Repository Transactions: Addressed issues within repository transactions to maintain data integrity and consistency.

Related issue/s

How to test

cd services/web/server
make install-dev
pytest -vv tests --cov=simcore_service_webserver --cov-config=../../../.coveragerc  tests/unit/**/*product*.py

Dev-ops

@pcrespov pcrespov self-assigned this Feb 28, 2025
@pcrespov pcrespov added the 🤖-do-not-merge (optional) blocks Mergify from merging the PR label Feb 28, 2025
@pcrespov pcrespov added this to the The Awakening milestone Feb 28, 2025
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Feb 28, 2025
@pcrespov pcrespov force-pushed the is7275/product-follow-up branch 10 times, most recently from 1db3b7a to 7f78cb5 Compare March 7, 2025 09:34
@ITISFoundation ITISFoundation deleted a comment from codecov bot Mar 7, 2025
@pcrespov pcrespov changed the title WIP: 🎨 Is7275/product follow up ♻️🎨 web-server: enhances product domain Mar 7, 2025
@pcrespov pcrespov marked this pull request as ready for review March 7, 2025 11:22
@codecov
Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 98.68421% with 4 lines in your changes missing coverage. Please review.

Project coverage is 89.48%. Comparing base (30a4d87) to head (6cb7179).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7294      +/-   ##
==========================================
+ Coverage   87.41%   89.48%   +2.07%     
==========================================
  Files        1693     1450     -243     
  Lines       65873    57544    -8329     
  Branches     1121      614     -507     
==========================================
- Hits        57580    51496    -6084     
+ Misses       7973     5877    -2096     
+ Partials      320      171     -149     
Flag Coverage Δ
integrationtests 65.35% <68.44%> (-0.04%) ⬇️
unittests 87.75% <98.68%> (+1.17%) ⬆️
Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 91.63% <100.00%> (-0.01%) ⬇️
pkg_notifications_library 84.57% <ø> (ø)
pkg_postgres_database 88.21% <96.29%> (-0.07%) ⬇️
pkg_service_integration 70.03% <ø> (ø)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.46% <ø> (ø)
agent 96.46% <ø> (ø)
api_server 90.68% <100.00%> (ø)
autoscaling 96.08% <ø> (ø)
catalog 92.10% <100.00%> (ø)
clusters_keeper 99.24% <ø> (ø)
dask_sidecar 91.25% <ø> (ø)
datcore_adapter 98.11% <ø> (ø)
director 76.59% <ø> (-0.19%) ⬇️
director_v2 91.27% <100.00%> (-0.03%) ⬇️
dynamic_scheduler 97.33% <ø> (ø)
dynamic_sidecar 90.12% <ø> (+0.04%) ⬆️
efs_guardian 89.79% <ø> (ø)
invitations 93.28% <ø> (ø)
osparc_gateway_server ∅ <ø> (∅)
payments 92.66% <100.00%> (ø)
resource_usage_tracker 89.11% <ø> (-0.11%) ⬇️
storage 86.86% <ø> (ø)
webclient ∅ <ø> (∅)
webserver 88.15% <98.85%> (+2.33%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30a4d87...6cb7179. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pcrespov pcrespov requested a review from bisgaard-itis March 7, 2025 11:23
@pcrespov pcrespov added 🤖-automerge marks PR as ready to be merged for Mergify and removed 🤖-do-not-merge (optional) blocks Mergify from merging the PR 🤖-automerge marks PR as ready to be merged for Mergify labels Mar 7, 2025
@pcrespov pcrespov force-pushed the is7275/product-follow-up branch from f59cbbb to fed33cd Compare March 7, 2025 15:44
@pcrespov pcrespov force-pushed the is7275/product-follow-up branch from fed33cd to 3cfc3ce Compare March 7, 2025 15:49
@pcrespov pcrespov enabled auto-merge (squash) March 7, 2025 16:08
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 7, 2025

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

thanks!

Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Very nice Pedro! Thanks a lot for the effort. I would like to discuss with you again about the design of these subdomains. I don't see where we don't align 🙂

@pcrespov pcrespov merged commit 9c10bb2 into ITISFoundation:master Mar 10, 2025
188 of 199 checks passed
@pcrespov pcrespov deleted the is7275/product-follow-up branch March 10, 2025 10:52
mrnicegyu11 pushed a commit to mrnicegyu11/osparc-simcore that referenced this pull request Mar 26, 2025
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Apr 15, 2025
56 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maintenance: Follow up on webserver.product domain

6 participants