Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 1, 2024

What do these changes do?

  • Addressed issues and enhancements from Can use tags in services and list services grouped by tags #6202:

    • 🐛 Bug Fixes:
      • Fixed: list_all no longer returns duplicate tags when user has two groups with access to them.
      • Fixed: get_tags now correctly returns the most permissive access rights by applying OR aggregation across all flags.
    • New Features:
      • Introduced a priority property for tags to enforce explicit ordering in listings.
    • ⚗️ 📝 Draft Work:
      • Initial draft of sharing interfaces for the tags repository (further development will be included in upcoming PRs).
  • ♻️ Refactor: Tags Repository:

    • Updated the repository to use the new asyncpg-based syntax with support from utils_repos helpers.
    • Added database layer support for the tags resource.
  • 🎨 Database Plugin Enhancements:

    • The web-server's db plugin now sets up an additional asyncpg engine
    • This change is in line with migrate code from aiopg to asyncpg #4529
    • Note that the app now initializes up to three PostgreSQL client engines (see test_all_pg_engines_in_app for details):
      1. aiopg engine (deprecated)
      2. asyncpg engine via sqlalchemy.ext.asyncio (new)
      3. Low-level asyncpg pool (deprecated)

Related issue/s

How to test

Driving tests:

cd packages/postgres-database
make install-dev
pytest -vv tests/test_utils_tags.py 

and

cd services/web/server
make install-dev
pytest -vv tests/unit/with_dbs/03/tags/test_tags.py

Dev-ops checklist

None

@pcrespov pcrespov added this to the MartinKippenberger milestone Oct 1, 2024
@pcrespov pcrespov added a:webserver webserver's codebase. Assigning the area is particularly useful for bugs a:database associated to postgres service and postgres-database package labels Oct 1, 2024
@pcrespov pcrespov self-assigned this Oct 1, 2024
@pcrespov pcrespov mentioned this pull request Oct 1, 2024
@codecov
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

Attention: Patch coverage is 88.42105% with 11 lines in your changes missing coverage. Please review.

Project coverage is 89.2%. Comparing base (cafbf96) to head (68c19c8).
Report is 615 commits behind head on master.

Files with missing lines Patch % Lines
...tabase/src/simcore_postgres_database/utils_tags.py 90.1% 2 Missing and 3 partials ⚠️
...er/src/simcore_service_webserver/tags/_handlers.py 63.6% 4 Missing ⚠️
.../server/src/simcore_service_webserver/tags/_api.py 92.3% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6479      +/-   ##
=========================================
+ Coverage    84.5%   89.2%    +4.6%     
=========================================
  Files          10    1325    +1315     
  Lines         214   55392   +55178     
  Branches       25     961     +936     
=========================================
+ Hits          181   49447   +49266     
- Misses         23    5792    +5769     
- Partials       10     153     +143     
Flag Coverage Δ
integrationtests 64.7% <40.4%> (?)
unittests 86.9% <88.4%> (+2.4%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../simcore_postgres_database/models/services_tags.py 100.0% <ø> (ø)
...abase/src/simcore_postgres_database/models/tags.py 100.0% <ø> (ø)
...ore_postgres_database/models/tags_access_rights.py 100.0% <ø> (ø)
...se/src/simcore_postgres_database/utils_tags_sql.py 94.1% <100.0%> (ø)
.../server/src/simcore_service_webserver/db/plugin.py 100.0% <100.0%> (ø)
...rver/src/simcore_service_webserver/tags/schemas.py 100.0% <100.0%> (ø)
.../server/src/simcore_service_webserver/tags/_api.py 92.3% <92.3%> (ø)
...er/src/simcore_service_webserver/tags/_handlers.py 73.7% <63.6%> (ø)
...tabase/src/simcore_postgres_database/utils_tags.py 92.3% <90.1%> (ø)

... and 1281 files with indirect coverage changes

@pcrespov pcrespov force-pushed the is6202/tags-db-layer branch from f25ce06 to f7c00b0 Compare October 2, 2024 13:48
@pcrespov pcrespov changed the title Is6202/tags db layer 🐛 Fixes duplicates in tags listings and new priority to enforce order Oct 2, 2024
@pcrespov pcrespov marked this pull request as ready for review October 2, 2024 15:37
@pcrespov pcrespov force-pushed the is6202/tags-db-layer branch from 86d1f9f to e482324 Compare October 2, 2024 17:37
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.

Thanks! 👍 I added some comments that came to mind while reviewing.

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

Thanks 👌 .

access_rights is now an OR aggregation, which is great because it won't break anything. Eventually we will need to expose access_rights and my_access_rights, so that the frontend can show with whom the tags are shared.

@pcrespov pcrespov force-pushed the is6202/tags-db-layer branch from e482324 to 98662e5 Compare October 3, 2024 13:20
@pcrespov pcrespov enabled auto-merge (squash) October 3, 2024 13:22
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 3, 2024

@pcrespov pcrespov merged commit fc8225a into ITISFoundation:master Oct 3, 2024
57 checks passed
@pcrespov pcrespov deleted the is6202/tags-db-layer branch October 3, 2024 17:28
mrnicegyu11 pushed a commit to mrnicegyu11/osparc-simcore that referenced this pull request Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:database associated to postgres service and postgres-database package a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants