Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 7, 2025

What do these changes do?

This PR implements TTL-based caching for authentication operations to improve performance and reduce database load.

🚀 New Features

  • Added aiocache with 120s TTL to ApiKeysRepository.get_user() method
  • Added aiocache with 120s TTL to UsersRepository.get_active_user_email() method

🔧 Configuration

  • Cache can be disabled via AIOCACHE_DISABLE=1 environment variable
  • Uses in-memory backend by default with automatic TTL cleanup
  • Added proper cache namespacing and noself=True configuration

Security Considerations

  • API secrets are never stored in cache secrets (only public API keys used for cache identification)
  • Short 120s TTL ensures revoked access doesn't persist long
  • Cache keys use only non-sensitive identifiers

Performance Impact

  • Reduces database calls for repeated authentication requests
  • Estimated memory usage: ~200-500 bytes per cached entry
  • Suitable for applications with <10K active users

Related issue/s

How to test

  • Added comprehensive cache effectiveness test in test_repository_api_keys.py
  • Test validates both performance improvements and database call reduction
  • Added test documentation covering memory implications and configuration optio
cd services/api-server
make install-dev
pytest -vv tests/unit/_with_db/authentication 

Dev-ops

@pcrespov pcrespov self-assigned this Oct 7, 2025
@pcrespov pcrespov added a:apiserver api-server service t:maintenance Some planned maintenance work labels Oct 7, 2025
@pcrespov pcrespov added this to the Cheops milestone Oct 7, 2025
@codecov
Copy link

codecov bot commented Oct 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.12%. Comparing base (ff25190) to head (dfaadbf).
⚠️ Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (ff25190) and HEAD (dfaadbf). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (ff25190) HEAD (dfaadbf)
unittests 32 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8474       +/-   ##
===========================================
- Coverage   87.10%   69.12%   -17.99%     
===========================================
  Files        1983      889     -1094     
  Lines       77285    39749    -37536     
  Branches     1333      175     -1158     
===========================================
- Hits        67322    27477    -39845     
- Misses       9564    12215     +2651     
+ Partials      399       57      -342     
Flag Coverage Δ
integrationtests 64.17% <ø> (+3.65%) ⬆️
unittests 91.90% <100.00%> (+5.57%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_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 76.63% <ø> (-8.26%) ⬇️
agent ∅ <ø> (∅)
api_server 91.90% <100.00%> (+0.12%) ⬆️
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.22% <ø> (-7.07%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 81.87% <ø> (-8.56%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 59.14% <ø> (-28.27%) ⬇️

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 ff25190...dfaadbf. 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 force-pushed the is8461/cache-public-api-auth branch from a5c896d to 0fbd190 Compare October 7, 2025 16:33
@pcrespov pcrespov marked this pull request as ready for review October 7, 2025 16:33
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 for quick action 👍

@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Oct 7, 2025
@pcrespov pcrespov enabled auto-merge (squash) October 7, 2025 16:51
@mergify
Copy link
Contributor

mergify bot commented Oct 7, 2025

🧪 CI Insights

Here's what we observed from your CI run for dfaadbf.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI system-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

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

@pcrespov pcrespov force-pushed the is8461/cache-public-api-auth branch from 9925491 to 829e3bd Compare October 8, 2025 07:15
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 8, 2025

@pcrespov pcrespov requested a review from wvangeit October 8, 2025 08:19
@pcrespov pcrespov merged commit 8b68319 into ITISFoundation:master Oct 8, 2025
91 of 95 checks passed
@pcrespov pcrespov deleted the is8461/cache-public-api-auth branch October 8, 2025 08:31
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.

[Maintenance]: Improve Get keys database endpoint

4 participants