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

[BUGFIX] Pose d'un verrou pour limiter la création de challenge en certification (PIX-16165) #11193

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

yaf
Copy link
Member

@yaf yaf commented Jan 22, 2025

🥞 Problème

Nous avons constaté qu'un certain nombre de certifications se retrouvent avec plus de 32 challenge.
Notre plus grosse piste est un soucis de concurrence au niveau de ce endpoint.

🥓 Proposition

Poser un verrou pour empêcher la création de challenge supplémentaire.

🧃 Remarques

Il y a aussi quelque chose a regarder cote Pix App, qui envoi en quelques millisecondes plusieurs appels sur ce endpoint.

😋 Pour tester

  • Pix certif, creer une session de certif et faire rentrer un candidat

  • Verifier qu'il peut passer sa certification (proposition de challenges) correctement sans erreur technique

  • [TECH] Des tests purement techniques seraient interessant pour essayer de verifier la situation de concurrence

@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@alexandrecoin alexandrecoin force-pushed the PIX-16165-concurrency-on-next-challenge branch from 09b48e7 to 7e72197 Compare January 23, 2025 08:00
@AndreiaPena AndreiaPena force-pushed the PIX-16165-concurrency-on-next-challenge branch from 7e72197 to 4fad05a Compare January 23, 2025 11:14
@alexandrecoin alexandrecoin force-pushed the PIX-16165-concurrency-on-next-challenge branch 3 times, most recently from 0fc6e51 to 4f708ae Compare January 24, 2025 10:23
Copy link
Contributor

@Steph0 Steph0 left a comment

Choose a reason for hiding this comment

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

Test technique en local en faisant

  • Deux containers (deux port different)
  • Un container en debugger apres le lock
  • Premier requete => arret sur debugger
  • Seconde requete => hang car peut pas prendre le lock
  • Relache du point d'arret => requete 1 se termine
  • On observe (une millisecond apres) immediatement que la requete 2 se debloque

Du coup pour moi l'objectif de la PR est OK ✅ ! Impossible de faire sur un meme assessmentId deux selections de challenge en meme temps

@alexandrecoin alexandrecoin force-pushed the PIX-16165-concurrency-on-next-challenge branch 2 times, most recently from 815f744 to e67c05f Compare January 24, 2025 13:17
@alexandrecoin alexandrecoin force-pushed the PIX-16165-concurrency-on-next-challenge branch from e67c05f to cd0b44d Compare January 24, 2025 13:30
@alexandrecoin alexandrecoin marked this pull request as ready for review January 24, 2025 13:38
@alexandrecoin alexandrecoin requested a review from a team as a code owner January 24, 2025 13:38
@alexandrecoin alexandrecoin force-pushed the PIX-16165-concurrency-on-next-challenge branch from cd0b44d to 1c81e28 Compare January 24, 2025 16:29
@alexandrecoin alexandrecoin added Tech Review OK Func Review OK PO validated functionally the PR and removed 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally labels Jan 24, 2025
In order to do so in the certification context, challenge selection
had to be migrated using an api
@yaf yaf force-pushed the PIX-16165-concurrency-on-next-challenge branch from 1c81e28 to db31f2b Compare January 27, 2025 08:51
@pix-service-auto-merge pix-service-auto-merge merged commit e4ecd12 into dev Jan 27, 2025
8 of 10 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the PIX-16165-concurrency-on-next-challenge branch January 27, 2025 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants