Skip to content
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

🐛Separate caching director call from user dependent service listing #3020

Merged

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented May 3, 2022

What do these changes do?

The caching of the list of services from the director was filtered with a user-dependent list of services available in the database. This explains the various errors in the catalog.

Related issue/s

fixes #3005

How to test

Checklist

@sanderegg sanderegg added this to the Macarons +1 milestone May 3, 2022
@sanderegg sanderegg requested a review from pcrespov as a code owner May 3, 2022 15:27
@sanderegg sanderegg self-assigned this May 3, 2022
@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #3020 (d851b99) into master (eec1f9e) will decrease coverage by 0.0%.
The diff coverage is n/a.

❗ Current head d851b99 differs from pull request most recent head 0a3dc36. Consider uploading reports for the commit 0a3dc36 to get more accurate results

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3020     +/-   ##
========================================
- Coverage    79.7%   79.7%   -0.1%     
========================================
  Files         693     693             
  Lines       29052   29049      -3     
  Branches     3745    3745             
========================================
- Hits        23158   23154      -4     
  Misses       5062    5062             
- Partials      832     833      +1     
Flag Coverage Δ
integrationtests 65.8% <0.0%> (+<0.1%) ⬆️
unittests 75.5% <0.0%> (-0.1%) ⬇️

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

Impacted Files Coverage Δ
...c/simcore_service_catalog/core/background_tasks.py 66.3% <0.0%> (-2.2%) ⬇️
...imcore_service_webserver/garbage_collector_core.py 68.2% <0.0%> (-1.3%) ⬇️
.../director/src/simcore_service_director/producer.py 61.6% <0.0%> (-0.9%) ⬇️
..._director_v2/modules/dynamic_sidecar/docker_api.py 87.1% <0.0%> (-0.5%) ⬇️
...2/src/simcore_service_director_v2/core/settings.py 98.7% <0.0%> (-0.1%) ⬇️
...ector_v2/modules/dynamic_sidecar/scheduler/task.py 81.9% <0.0%> (ø)
...es/dynamic_sidecar/docker_service_specs/sidecar.py 78.5% <0.0%> (ø)
..._director_v2/modules/dynamic_sidecar/client_api.py 78.4% <0.0%> (+1.0%) ⬆️
...tor_v2/modules/dynamic_sidecar/scheduler/events.py 93.8% <0.0%> (+1.6%) ⬆️
.../simcore_service_catalog/services/access_rights.py 81.2% <0.0%> (+2.5%) ⬆️
... and 1 more

@KZzizzle
Copy link
Contributor

KZzizzle commented May 3, 2022

thanks for the notice! (and congrats on sonarcloud)

@sanderegg sanderegg force-pushed the bugfix/3005_catalog_key_error branch from a1a7ac8 to 6c2971b Compare May 3, 2022 16:14
Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

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

Thanks a lot!
Please consider making DIRECTOR_CACHING_TTL adjustable via env-vars, and if DIRECTOR_CACHING_TTL=0 maybe caching could be disabled? Just a suggestion, the former would be easier I guess :)

thanks for taking care of this :)

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.

👍

@sanderegg sanderegg force-pushed the bugfix/3005_catalog_key_error branch from 6c2971b to ab42f3f Compare May 4, 2022 07:32
@sanderegg sanderegg force-pushed the bugfix/3005_catalog_key_error branch from ab42f3f to 0a3dc36 Compare May 4, 2022 14:33
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 4, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sanderegg sanderegg merged commit 2a3913b into ITISFoundation:master May 4, 2022
@sanderegg sanderegg deleted the bugfix/3005_catalog_key_error branch May 4, 2022 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catalog KeyError: Caching issues? - portal links fail
5 participants