Skip to content

♻️🗑️ api-server upgraded to use asyncpg #7598

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

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Apr 28, 2025

TL;DR

Mainly upgrades all code in api-server to use asyncpg for the database and drops all dependencies to aiopg

What do these changes do?

This pull request introduces significant changes to the services/api-server module, focusing on refactoring the database layer, removing unused dependencies, and improving the structure and maintainability of the codebase. The most important changes include replacing aiopg with sqlalchemy for database interactions, restructuring the repository layer, and cleaning up unused database-related code.

Database Refactoring:

  • Replaced aiopg with sqlalchemy for database connections and interactions, including changes to BaseRepository to use AsyncEngine instead of aiopg.sa.Engine. ([[1]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-be157b2a3756831842f4e1c8d75f9ef87e9de3c9fb58137911c646fc5bd62ad8L3-R3), [[2]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-be157b2a3756831842f4e1c8d75f9ef87e9de3c9fb58137911c646fc5bd62ad8L12-R12))
  • Introduced a new get_engine function in clients/postgres.py to manage database engine retrieval and lifecycle, replacing the previous get_asyncpg_engine function. ([[1]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-6a4b619833ff57a298b60f5bca615b25dd02b3e60799c231a8af817a9b5c41abR1-R42), [[2]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-5c7b743926ea6fabcf9c771fc4d915e889751ffb982a8ad42f52396f0b4ec6cbL3-L53))
  • Simplified database connection setup and teardown by introducing setup_postgres in clients/postgres.py and removing redundant methods in events.py. ([[1]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-6a4b619833ff57a298b60f5bca615b25dd02b3e60799c231a8af817a9b5c41abR1-R42), [[2]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-c427b44a791cbdf2212030e64e52a3f5fe10d62e09a63bb0aeb7a03f93f4a580R10), [[3]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-05a97611ca8826d2affafa810b68f7e60916ef28b23dd2b727c3628c7cc7a42bL7-R21))

Repository Layer Restructuring:

  • Moved repository files from db/repositories to a new repository directory, reflecting a more generic structure. ([[1]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-be157b2a3756831842f4e1c8d75f9ef87e9de3c9fb58137911c646fc5bd62ad8L3-R3), [[2]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-5ae7d64cdb0586a86a289ecd72276112bf66fad511094e4e29b89baac3914ebbR8-L9))
  • Updated repository methods to use sqlalchemy's AsyncConnection for database queries and introduced pass_or_acquire_connection for connection management. ([[1]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-5ae7d64cdb0586a86a289ecd72276112bf66fad511094e4e29b89baac3914ebbR23-R49), [[2]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-cd30ba2b22f880996a8d13671b2dbf7d84f266c700664582cc5eeb9ba5bc2864R1-R30))

Dependency Cleanup:

  • Removed aiopg and related dependencies (async-timeout, aiopg[sa]) from requirements files, as they are no longer needed. ([[1]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-3daaa81f339b8dda16afde0025886f5125164e6e1f882aad59941fe5ca66f50aL20), [[2]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-cd7fafa2f9b4f57ff1861bba8d3ad7f4d35c9a96e3e32a1401ee8ee84271d9baL58-L59), [[3]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-cd7fafa2f9b4f57ff1861bba8d3ad7f4d35c9a96e3e32a1401ee8ee84271d9baL87-L88), [[4]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-cd7fafa2f9b4f57ff1861bba8d3ad7f4d35c9a96e3e32a1401ee8ee84271d9baL516-R512), [[5]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-cd7fafa2f9b4f57ff1861bba8d3ad7f4d35c9a96e3e32a1401ee8ee84271d9baL788))

Code Simplification:

  • Removed unused or redundant code, such as the GroupsExtraPropertiesRepository and its associated methods. ([services/api-server/src/simcore_service_api_server/db/repositories/groups_extra_properties.pyL1-L27](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-03bf6a7e1bcf87c696c6aace88caf6797da82d126de396f4e13f7496b1504130L1-L27))
  • Refactored inline conditionals and logging for better readability and maintainability. ([[1]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-f7c1e711d8a8895038499f64e561dc6357f3c1a0f96714c027d46eb737fec51cL25-R27), [[2]](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-f7c1e711d8a8895038499f64e561dc6357f3c1a0f96714c027d46eb737fec51cL49-R49))

Test Adjustments:

  • Updated test configurations to align with the new database structure and removed unused imports. ([services/api-server/tests/unit/_with_db/conftest.pyL11-R11](https://github.com/ITISFoundation/osparc-simcore/pull/7598/files#diff-80bb13f6a26e5399ead0832843cc44feac841d56767f447ffd42ad15e53b3219L11-R11))

Follow-Up Tasks

Following this PR, the following improvements should be addressed:

  • Upgrade the app to use the new lifespan protocol
  • Remove optional PostgreSQL and RabbitMQ configuration settings (this service cannot work without these services)
  • Simplify the database fixtures

Related issue/s

How to test

cd services/api-server
make install-dev
make test-dev-unit

Dev-ops

None

@pcrespov pcrespov added the a:apiserver api-server service label Apr 28, 2025
@pcrespov pcrespov added this to the Pauwel Kwak milestone Apr 28, 2025
@pcrespov pcrespov self-assigned this Apr 28, 2025
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Apr 28, 2025
Copy link

codecov bot commented Apr 28, 2025

Codecov Report

Attention: Patch coverage is 92.85714% with 6 lines in your changes missing coverage. Please review.

Project coverage is 69.56%. Comparing base (168a1d6) to head (532647b).
Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (168a1d6) and HEAD (532647b). Click for more details.

HEAD has 30 uploads less than BASE
Flag BASE (168a1d6) HEAD (532647b)
unittests 31 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #7598       +/-   ##
===========================================
- Coverage   87.68%   69.56%   -18.13%     
===========================================
  Files        1776      779      -997     
  Lines       68432    35528    -32904     
  Branches     1125      170      -955     
===========================================
- Hits        60007    24715    -35292     
- Misses       8118    10755     +2637     
+ Partials      307       58      -249     
Flag Coverage Δ
integrationtests 65.18% <ø> (+0.06%) ⬆️
unittests 91.40% <92.85%> (+4.52%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 77.41% <ø> (-8.25%) ⬇️
agent ∅ <ø> (∅)
api_server 91.40% <92.85%> (+0.13%) ⬆️
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.38% <ø> (-13.00%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 89.06% <ø> (-1.10%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 59.18% <ø> (-26.87%) ⬇️

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 168a1d6...532647b. 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 changed the title WIP: I4529/api server upgrade to asyncpg ♻️🗑️ api-server upgraded to use asyncpg Apr 28, 2025
@pcrespov pcrespov marked this pull request as ready for review April 28, 2025 12:00
Copy link
Contributor

@Copilot Copilot AI left a 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 upgrades the api-server to use asyncpg via SQLAlchemy’s AsyncEngine, removing all dependencies on aiopg and cleaning up associated code. Key changes include refactoring the database and repository layers, updating test fixtures and dependency injections to match the new asyncpg usage, and removing unused/deprecated code.

Reviewed Changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit/test_api_solver_jobs.py Updated test parameter types to use MockRouter and removed obsolete mocks.
tests/unit/conftest.py Adjusted imports and updated fixture references to reflect new repository paths.
tests/unit/api_solvers/test_api_routers_solvers_jobs.py Refined type annotations and removed unused mock fixtures.
tests/unit/_with_db/conftest.py Migrated from connection-based operations to an async_engine approach with async context managers.
src/simcore_service_api_server/repository/*.py Introduced repositories with AsyncEngine integration and removed legacy modules.
src/simcore_service_api_server/db/* Removed deprecated database event modules and repository files.
src/simcore_service_api_server/core/* Updated app event handlers and application lifecycle management.
src/simcore_service_api_server/clients/postgres.py Added new postgres client functionality with async startup/shutdown event handlers.
src/simcore_service_api_server/api/dependencies/* Updated dependency injection to use get_engine and switched repository imports accordingly.
Files not reviewed (2)
  • services/api-server/requirements/_base.in: Language not supported
  • services/api-server/requirements/_base.txt: Language not supported
Comments suppressed due to low confidence (1)

tests/unit/test_api_solver_jobs.py:393

  • [nitpick] The type annotation for 'mocked_solver_job_outputs' is set to None; consider using a proper mock type (e.g. MockRouter or a dedicated interface) for consistency with other similar parameters.
mocked_solver_job_outputs: None,

@pcrespov pcrespov enabled auto-merge (squash) April 28, 2025 12:01
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, looks good

Copy link
Member

@sanderegg sanderegg 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
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍

@pcrespov
Copy link
Member Author

@mergify queue

Copy link
Contributor

mergify bot commented Apr 28, 2025

queue

🛑 The pull request has been merged manually

The pull request has been merged manually at 6c947eb

@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Apr 28, 2025
@pcrespov pcrespov merged commit 6c947eb into ITISFoundation:master Apr 28, 2025
93 of 96 checks passed
@pcrespov pcrespov deleted the i4529/api-server-upgrade-to-asyncpg branch April 28, 2025 14:20
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request May 8, 2025
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖-automerge marks PR as ready to be merged for Mergify a:apiserver api-server service t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants