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

[Demandeur d’emploi] Reprise de stock des critères certifiés #4439

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

celine-m-s
Copy link
Collaborator

@celine-m-s celine-m-s commented Jul 17, 2024

🤔 Pourquoi ?

La PR #4524 permet d'appeler l'API Particulier pour certifier certains critères sélectionnés dans le cadre des diagnostics d'éligibilité.
Il convient maintenant de certifier les critères sélectionnés dans les diagnostics des six derniers mois.

🍰 Comment ?

L'API ne mentionne pas de limite et n'explique pas les nombreuses 503 qui apparaissent plus souvent que les 429. J'ai contacté l'équipe pour en savoir plus mais, en attendant, c'est plus sûr de laisser tourner jusqu'à ce que l'API nous ait fourni toutes les informations demandées.
En effectuant quelques tests, j'ai l'impression que les erreurs 503 sont stables car je l'ai eue systématiquement avec certains bénéficiaires sans pour autant en trouver de raison vraiment pertinente. Je ne sais pas s'il s'agit d'un souci avec les données, d'une limite soudainement imposée ou d'une panne.
C'est pourquoi je propose qu'une commande de gestion tourne tous les soirs pendant quelques semaines afin d'appeler l'API et de récupérer le stock.

Autre point : le script est beaucoup plus rapide lorsque les appels sont lancés en parallèle (c'est logique) mais il certifie moins de critères car l’API emet plus d’erreurs. Les codes 429, 503 et 504 ne sont pas retentés. J'ai donc ajouté une option pour le lancer en synchrone, au cas où, même si je privilégie l'asynchrone car le script est censé tourner tous les soirs. La reprise se fera progressivement.

La rapidité d'exécution du script dépend beaucoup des codes 500 car les 200 sont plus lentes à répondre. À titre indicatif, voici les résultats sur une base réelle.

# Synchronously in dry run
# Total criteria to be certified: 25356
# Not found: 4439
# Server errors: 725
# That's 78.37% users found.
# Management command succeeded in 33919.55 seconds // about 9h 25min.

# Asynchronously in dry run
# Total criteria to be certified: 23657
# Not found: 1153
# Server errors: 16976
# That's 21.79% users found.
# Management command succeeded in 6448.19 seconds // about 1h and 50 minutes.

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?

🏝️ Comment tester

Avec une base réelle en local.

@celine-m-s celine-m-s added no-changelog Ne doit pas figurer dans le journal des changements. ajouté Ajouté dans le changelog. labels Jul 17, 2024
@celine-m-s celine-m-s self-assigned this Jul 17, 2024
@celine-m-s celine-m-s removed the ajouté Ajouté dans le changelog. label Jul 17, 2024
@celine-m-s celine-m-s force-pushed the celinems/brsa_mngt_cmd branch 2 times, most recently from b458311 to d53c328 Compare October 16, 2024 10:12
@celine-m-s celine-m-s marked this pull request as ready for review October 16, 2024 10:17
@celine-m-s celine-m-s changed the title Profil salarié : certification critère BRSA : reprise de stock [Profil salarié] certification critère BRSA : reprise de stock Oct 16, 2024
@celine-m-s celine-m-s changed the title [Profil salarié] certification critère BRSA : reprise de stock [Demandeur d’emploi] Certification critère BRSA : reprise de stock Oct 16, 2024
@celine-m-s celine-m-s changed the title [Demandeur d’emploi] Certification critère BRSA : reprise de stock [Demandeur d’emploi] Reprise de stock des critères certifiés Oct 16, 2024
@celine-m-s celine-m-s force-pushed the celinems/brsa_mngt_cmd branch 2 times, most recently from 992dcb3 to 00109f6 Compare October 16, 2024 10:45
@tonial
Copy link
Contributor

tonial commented Oct 17, 2024

Ça manque un peu d'un test pour une commande qu'on va laisser tourner régulièrement :)

Et ça permettra sans doute de plus facilement debugger le fait que l'asynchrone semble ne pas marcher bien avec tenacity

@celine-m-s
Copy link
Collaborator Author

@tonial J'ai revu le script et ajouté des tests car tous les cas n'étaient pas couverts. Merci de l'avoir vu !
En effet, il manque le test sur la commande en elle-même. Il est trop tard et je n'ai plus le temps de la faire, je vous laisse donc reprendre le script, effectuer les modifications nécessaires et le passer en prod.

Copy link

@francoisfreitag francoisfreitag self-assigned this Oct 21, 2024
@celine-m-s
Copy link
Collaborator Author

Merci d’avoir repris le sujet François !

@celine-m-s
Copy link
Collaborator Author

J’ai reçu plus d’infos sur le rate limiting :

Bonjour bonjour
On a un rate limiting mais il est relativement généreux : Un plafond général par IP de 1000 requêtes/minute ; Une volumétrie par jeton : 20 requêtes/secondes.
Cf https://particulier.api.gouv.fr/developpeurs

Normalement on ne fais pas les requêtes en batch on est pas faits pour ça mais si tu peux caler dans les limites du rate-limiting c'est pas un soucis

https://mattermost.incubateur.net/betagouv/pl/6ieqbhir4pn55j47h8994g7iew

@francoisfreitag francoisfreitag marked this pull request as draft October 22, 2024 16:19
@francoisfreitag francoisfreitag force-pushed the celinems/brsa_mngt_cmd branch 2 times, most recently from 28dfe35 to f2d5044 Compare October 23, 2024 09:08
@francoisfreitag francoisfreitag force-pushed the celinems/brsa_mngt_cmd branch 4 times, most recently from e8a4dac to 70cbc00 Compare October 23, 2024 09:13
@francoisfreitag francoisfreitag marked this pull request as ready for review October 23, 2024 09:14
@francoisfreitag
Copy link
Contributor

Je vais probablement merger en l’état, histoire d’avoir une première passe. Pas mal de modifications et correctifs sur l’intégration de l’API arriveront ensuite.

@francoisfreitag
Copy link
Contributor

La commande est en cours d’exécution sur une fast machine avec le code de 1d6f752.

Certify stored selected criteria each day at noon

The Particulier API returns many 503 without any useful
explanation.

Enhance tests to reflect real world API returns
@francoisfreitag
Copy link
Contributor

Pour info, la version finale du script a tourné avec :

diff --git a/itou/users/management/commands/certify_selected_administrative_criteria.py b/itou/users/management/commands/certify_selected_administrative_criteria.py
index 78c460237..25895c0ee 100755
--- a/itou/users/management/commands/certify_selected_administrative_criteria.py
+++ b/itou/users/management/commands/certify_selected_administrative_criteria.py
@@ -82,9 +82,9 @@ class Command(BaseCommand):
             f"Candidats à certifier pour le modèle {SelectedAdministrativeCriteriaModel.__name__}: {users_count}"
         )

-        chunks_total = ceil(total_criteria / 1000)
+        chunks_total = ceil(total_criteria / 50)
         chunks_count = 0
-        for criteria_pk_subgroup in chunks(criteria_pks, 1000):
+        for criteria_pk_subgroup in chunks(criteria_pks, 50):
             criteria = SelectedAdministrativeCriteriaModel.objects.filter(pk__in=criteria_pk_subgroup).select_related(
                 "administrative_criteria",
                 "eligibility_diagnosis__job_seeker",
@@ -97,6 +97,8 @@ class Command(BaseCommand):
                 for criterion in criteria:
                     criterion.certify(client, save=False)
                     data_returned_by_api = criterion.data_returned_by_api
+                    if data_returned_by_api is None:
+                        continue

                     if data_returned_by_api.get("status"):
                         total_criteria_with_certification += 1
diff --git a/itou/utils/apis/api_particulier.py b/itou/utils/apis/api_particulier.py
index 4e8c3bbd2..2ddf612ae 100644
--- a/itou/utils/apis/api_particulier.py
+++ b/itou/utils/apis/api_particulier.py
@@ -108,7 +108,7 @@ def revenu_solidarite_active(client, job_seeker):
         "start_at": None,
         "end_at": None,
         "is_certified": None,
-        "raw_response": "",
+        "raw_response": None,
     }
     try:
         response_data = _request(client, "/v2/revenu-solidarite-active", job_seeker)

@francoisfreitag
Copy link
Contributor

Dans un until $(./manage.py ...); do echo Retry; done 😇, #déso

@celine-m-s
Copy link
Collaborator Author

Quel est le résultat du script pour l’instant ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Ne doit pas figurer dans le journal des changements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants