Produits Entr'ouvert » Chrono

Et donc le coupable, « Copier le paramétrage du guichet »...

En plus du patch pour éviter cette duplication, il va falloir une migration qui ajoute une contrainte d'unicité sur le slug de la source d'exception, et vire les doublons au préalable.

Side effect, ça fait des exceptions qu'on ne peut plus désactiver, #47915.

Valentin Deniaud a écrit :

je m'y repenche demain.

C'est un comportement documenté et je n'ai pas l'impression qu'il y ait de contournement, https://docs.djangoproject.com/en/1.11/ref/migration-operations/#django.db.migrations.operations.RunPython :

you should avoid combining schema changes and RunPython operations in the same migration or you may hit errors like OperationalError: cannot ALTER TABLE "mytable" because it has pending trigger events

Mais en fait c'est un faux problème que je me posais : comme la première migration assure l'unicité, et que la deuxième ajoute la contrainte, je me disais qu'un doublon pouvait très hypothétiquement être créé dans l'intervalle et faire échouer le truc. Mais un doublon ne peut plus être créé, c'est assuré par le premier patch. Donc tout va bien.

Valentin Deniaud a écrit :

Et en tapant ça je m'aperçois que le deuxième point n'est pas spécifique aux exceptions jours fériés, c'est valable avec toutes les sources (import d'un ICS -> duplication -> suppression de l'ICS dans le guichet dupliqué (rien ne se passe) -> suppression de l'ICS dans le fichier initial (exception supprimées dans les deux guichets).

Mais ça s'autocorrige tout seul au bout d'une heure avec la commande qui synchronise les ICS (cycle qui supprime/remet les exceptions), c'est pour ça que c'est passé sous les radars. Pas besoin de migration qui rattrape, donc.

0001 : ok
0002 : ok
0003 : ok, juste je me demande si prendre le premier des doublons d'une source ayant des exceptions est suffisant, par rapport à leur état (source.enabled)

Note pour moi : pour le 0003 il faut juste se mettre dans l'idée quand on lit qu'en fait les sources passent leur temps à effacer toutes les exceptions qui leur sont liées à chaque fois qu'elles se mettent à jour, donc en fait on a très peu de chance de trouver des exceptions incohérentes (lièe au mauvais bureau, j'ai eu un peu du mal à comprendre pourquoi on pouvait en faire aussi peu) et supprimer les sources en trop suffit (aucune source ne manque et il n'y a pas de doublon de source pour le type de source "ics")

On peut peut-être remplacé ça par un update_or_create() maintenant qu'on a un index d'unicité :

            try:
                source = TimePeriodExceptionSource.objects.get(desk=self, settings_slug=slug)
            except TimePeriodExceptionSource.DoesNotExist:
                source = TimePeriodExceptionSource.objects.create(
                    desk=self, settings_slug=slug, enabled=False
                )

Thx pour la relecture.

Benjamin Dauvergne a écrit :

0003 : ok, juste je me demande si prendre le premier des doublons d'une source ayant des exceptions est suffisant, par rapport à leur état (source.enabled)

Quand tu dupliques une première fois, tu te retrouves avec deux sources enabled, l'une créée par l'import lors du save(), l'autre par la duplication. On veut garder la première, qui a correctement créé ses exceptions, et pas la seconde. Donc on voit qu'on ne peut pas se fier à enabled pour prendre des décisions ici.
Lors des duplications suivantes, on peut se retrouver avec plusieurs sources enabled qui ont des exceptions, il suffit d'en choisir une au pif.

On peut peut-être remplacé ça par un update_or_create() maintenant qu'on a un index d'unicité :

En vrai je relis cette ligne et je ne comprends pas pourquoi il n'y a pas un get_or_create (certes il n'y a pas d'index d'unicité mais je ne pense pas que ce soit la raison vu que le même pb se pose avec get tout court). Par contre update_or_create je pense pas, on ne veut pas mettre à jour la valeur de enabled. Trop frileux pour y toucher ici en tout cas.

Valentin Deniaud a écrit :

Benjamin Dauvergne a écrit :

0003 : ok, juste je me demande si prendre le premier des doublons d'une source ayant des exceptions est suffisant, par rapport à leur état (source.enabled)

Quand tu dupliques une première fois, tu te retrouves avec deux sources enabled, l'une créée par l'import lors du save(), l'autre par la duplication. On veut garder la première, qui a correctement créé ses exceptions, et pas la seconde. Donc on voit qu'on ne peut pas se fier à enabled pour prendre des décisions ici.
Lors des duplications suivantes, on peut se retrouver avec plusieurs sources enabled qui ont des exceptions, il suffit d'en choisir une au pif.

• duplication comme je décris, le guichet dupliqué a une source enabled avec des exceptions, l'autre enabled sans
• quelqu'un vient désactiver la source avec des exceptions
• la migration risque de garder la source enabled qui n'a pas d'exceptions, alors qu'on aurait voulu celle dans un état cohérent.

J'ajoute un petit if qui gère ça et je pousse.