Produits Entr'ouvert » Passerelle

Comme tu modifies une demandes existante, pour éviter de te marcher dessus avec le job d'envoi il faudrait enfermer tout ça dans une transaction (transaction.atomic) et faire un select_for_update() si le .get() réussit.

Il faudrait faire pareil dans le job (pour que les deux méthodes verrouillent l'objet WcsRequest en base).

il faudrait enfermer tout ça dans une transaction (transaction.atomic) et faire un select_for_update() si le .get() réussit.

J'ai fait ça (create_intervention est déjà en atomic, via un décorateur).

Il faudrait faire pareil dans le job (pour que les deux méthodes verrouillent l'objet WcsRequest en base).

Fait.

Tu ne peux pas faire de .select_for_update() en dehors d'une transaction :

        wcs_request = self.wcs_requests.select_for_update().get(pk=kwargs['pk'])
        with atomic():

Tu ne peux pas faire de .select_for_update() en dehors d'une transaction

Compris, c'était bien indiqué dans la doc django, mais l'exemple n'était pas clair
https://docs.djangoproject.com/fr/4.0/ref/models/querysets/#select-for-update
(c'est pas évident ce patch, parce que je ne saurais pas écrire un test de non régression).

Nouveau patch 0004 qui modifie ça :

     def create_intervention_job(self, *args, **kwargs):
-        wcs_request = self.wcs_requests.select_for_update().get(pk=kwargs['pk'])
         with atomic():
+            wcs_request = self.wcs_requests.select_for_update().get(pk=kwargs['pk'])
             after_timestamp = None

Nicolas Roche a écrit :

Tu ne peux pas faire de .select_for_update() en dehors d'une transaction

Compris, c'était bien indiqué dans la doc django, mais l'exemple n'était pas clair
https://docs.djangoproject.com/fr/4.0/ref/models/querysets/#select-for-update
(c'est pas évident ce patch, parce que je ne saurais pas écrire un test de non régression).

Nouveau patch 0004 qui modifie ça :
[...]

Je ne sais pas ce que tu appelles non régression ici, si c'est testé l'exclusion du au verrou par select_for_update() ça peut se faire avec transactional_db et de threads/processs mais c'est pas vraiment utile. Si le code passe sur le .select_for_update() c'est verrouillé, on sait que ailleurs si on essaie de verrouiller ça ne passera pas, en général si tu n'as à poser qu'un verrou aucun souci n'est possible (à partir de deux verrous tu peux avoir des soucis).

Ici ton test est bien, il teste qu'on reprend bien une demande existante en cas de rejeu et c'est verrouillé, y a pas d'autre moyen.

Si on avait besoin de performance ici Pierre D. te dirait de limiter le select_for_update() au cas created == False mais je ne pense pas qu'on en ait besoin (dans le cas où l'objet est créé il est invisible en dehors de la transaction jusqu'au commit).

commit a8530e2806c0d31c454c4a80a1f8f912c1fbaaa8
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Sat Jul 23 19:10:41 2022 +0200

    toulouse_smart: allow to redo a failed intervention (#62012)

commit 5657d2896b7607ace4b70fd563d545ac7a5bd5e1
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Sat Jul 23 19:09:20 2022 +0200

    toulouse_smart: factorize wcs_request response (#62012)

commit 5f695ccd4aa516e84c2fda229cdea1b8094d13ce
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Sat Jul 23 11:46:47 2022 +0200

    toulouse_smart: check media path on tests (#62012)

commit 4543200ddf09fde0bc73aba0eb87632be06eb541
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Sat Jul 23 11:40:20 2022 +0200

    toulouse_smart: check response error on tests (#62012)