Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented May 29, 2024

What do these changes do?

While starting a jupyter (in master), the variable substitution component triggers a callback to auto-generate a valid api-key for the current user. This callback raises

 File "/home/scu/.venv/lib/python3.10/site-packages/simcore_service_director_v2/modules/osparc_variables/_api_auth.py", line 40, in _get_or_create_data
    return await create_api_key_and_secret(
  File "/home/scu/.venv/lib/python3.10/site-packages/simcore_service_director_v2/modules/osparc_variables/_api_auth_rpc.py", line 28, in create_api_key_and_secret
    result = await rpc_client.request(
  File "/home/scu/.venv/lib/python3.10/site-packages/servicelib/rabbitmq/_client_rpc.py", line 96, in request
    return await asyncio.wait_for(awaitable, timeout=timeout_s)
  File "/usr/local/lib/python3.10/asyncio/tasks.py", line 445, in wait_for
    return fut.result()
  File "/home/scu/.venv/lib/python3.10/site-packages/aio_pika/patterns/rpc.py", line 413, in call
    return await future
servicelib.rabbitmq._errors.RPCServerError: While running method 'create_api_keys' raised 'psycopg2.errors.UniqueViolation': 'duplicate key value violates unique constraint "display_name_userid_uniqueness"
DETAIL:  Key (display_name, user_id)=(__auto_c52f362c-10cc-55b9-9539-82d97856bf06, 530) already exists.
'

Manual exploratory testing suggested that this error is produced by a race condition when executing get and create operations. Note that get_or_create operation was implemented in the directorv2 calls to get and create function are connected via RPC -> service -> repo.

  • 🐛 Implements get_or_create as a single transaction in the api_keys._db repository
  • 🗑️ retires unused api_keys_manager.py

Related issue/s

How to test

Driving tests

cd services/web/server
make install-dev
pytest -vv tests/unit -k test_get_or_create_api_key

Dev-ops checklist

@pcrespov pcrespov self-assigned this May 29, 2024
@pcrespov pcrespov added this to the Leeroy Jenkins milestone May 29, 2024
@codecov
Copy link

codecov bot commented May 29, 2024

Codecov Report

Attention: Patch coverage is 95.34884% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 85.1%. Comparing base (cafbf96) to head (d5bed17).
Report is 232 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5890      +/-   ##
=========================================
+ Coverage    84.5%   85.1%    +0.5%     
=========================================
  Files          10     561     +551     
  Lines         214   28228   +28014     
  Branches       25     205     +180     
=========================================
+ Hits          181   24032   +23851     
- Misses         23    4145    +4122     
- Partials       10      51      +41     
Flag Coverage Δ
integrationtests 63.6% <46.5%> (?)
unittests 86.4% <95.3%> (+1.8%) ⬆️

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

Files Coverage Δ
...rc/simcore_service_director_v2/core/application.py 96.2% <100.0%> (ø)
...s/dynamic_sidecar/scheduler/_core/_events_utils.py 92.6% <ø> (ø)
..._director_v2/modules/osparc_variables/_api_auth.py 100.0% <100.0%> (ø)
...ver/src/simcore_service_webserver/api_keys/_api.py 100.0% <100.0%> (ø)
...rver/src/simcore_service_webserver/api_keys/_db.py 95.0% <100.0%> (ø)
...ector_v2/modules/osparc_variables/_api_auth_rpc.py 76.9% <66.6%> (ø)
...ver/src/simcore_service_webserver/api_keys/_rpc.py 96.0% <80.0%> (ø)

... and 564 files with indirect coverage changes

@sonarqubecloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.2% Duplication on New Code

See analysis details on SonarCloud

@pcrespov pcrespov added a:webserver webserver's codebase. Assigning the area is particularly useful for bugs a:director-v2 issue related with the director-v2 service labels May 29, 2024
@pcrespov pcrespov changed the title 🐛 Fix/api keys duplicate 🐛 Fixes api-keys unique constraint violation May 29, 2024
@pcrespov pcrespov marked this pull request as ready for review May 29, 2024 15:01
@pcrespov pcrespov enabled auto-merge (squash) May 29, 2024 15:15
@pcrespov pcrespov changed the title 🐛 Fixes api-keys unique constraint violation 🐛 Fixes api-keys unique constraint violation (🚨) May 29, 2024
Copy link
Contributor

@wvangeit wvangeit 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 for finding/solving that.

@pcrespov pcrespov merged commit 030f126 into ITISFoundation:master May 29, 2024
@pcrespov pcrespov deleted the fix/api-keys-duplicate branch May 29, 2024 15:48
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jun 12, 2024
30 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:director-v2 issue related with the director-v2 service 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.

3 participants