Produits Entr'ouvert » Chrono

ça a l'air d'etre lié à un bug dans IntervalTree https://github.com/chaimleib/intervaltree/issues/41. J'essaie de m'en assurer.

Josué Kouka a écrit :

ça a l'air d'etre lié à un bug dans IntervalTree https://github.com/chaimleib/intervaltree/issues/41. J'essaie de m'en assurer.

Bon je confirme que c'est bien ce bug. Pour ne pas que ça bloque sur la prod j'ai fait à ce que aucune exception ne soit levée lorsqu'un Interval à supprimer n'est pas trouvé en posant le "hotfix" ci dessous dans /usr/lib/python2.7/dist-packages/intervaltree/node.py

236         if self.center_hit(interval):
237             #if trace: print('Hit at {}'.format(self.x_center))                                                                                                                                                                                                             
238             should_raise_error = False # hotfix
239             if not should_raise_error and interval not in self.s_center:
240                 done.append(1)

Pour m'assurer que tout marche, j'ai fait des tests via le shell django du tenant de meaux et via le formulaire mise en ligne sur la prod de Meaux (j'ai évidement annulé mes rendez-vous).

Il y'a ce commit https://github.com/chaimleib/intervaltree/commit/2d1b463db7e9dc81a74dc50c24c32fab4cb21d7f d'il y'a 4 jours, mais il semble ne pas marcher dans la cas de meaux. Je continue de regarder coté de la lib pour voir si je peux soumettre un patch aussi...

Quelle situation qui n'est pas dans les tests de chrono produit ce bogue ? Selon moi il faut commencer par ajouter un test dans chrono qui lève le problème... non ?

Thomas Noël a écrit :

Quelle situation qui n'est pas dans les tests de chrono produit ce bogue ? Selon moi il faut commencer par ajouter un test dans chrono qui lève le problème... non ?

Yep vraie. J'essaie de sortir le jeu de tests en me basant sur le les données de meaux.

(En passant, le patch passant c'est https://github.com/chaimleib/intervaltree/pull/63/commits/7898b6767b6e00f9ef856f2fb3831f7c98a469f4)

Pour se passer d'intervaltree.

L'approche me va bien mais il reste quand même à ajouter un test unitaire correspondant à la situation à Meaux et quelques tests pour porter la couverture de chrono/interval.py à 100% (quelques tests ou quelques suppressions de lignes, par exemple le constructeur d'Intervals n'est jamais appelé avec un paramètre).

Pendant ce temps…

Internal Server Error: /api/agenda/agenda2/meetings/3pers/datetimes/
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/django/core/handlers/base.py", line 132, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib/python2.7/dist-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/rest_framework/views.py", line 466, in dispatch
    response = self.handle_exception(exc)
  File "/usr/lib/python2.7/dist-packages/rest_framework/views.py", line 463, in dispatch
    response = handler(request, *args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/chrono/api/views.py", line 210, in get
    open_slots, all_time_slots = get_all_slots(agenda, meeting_type)
  File "/usr/lib/python2.7/dist-packages/chrono/api/views.py", line 83, in get_all_slots
    open_slots_by_desk[event.desk_id].remove_overlap(event.start_datetime, event.end_datetime)
  File "/usr/lib/python2.7/dist-packages/intervaltree/intervaltree.py", line 473, in remove_overlap
    self.remove(iv)
  File "/usr/lib/python2.7/dist-packages/intervaltree/intervaltree.py", line 360, in remove
    raise ValueError
ValueError

Vient d'avoir lieu sur /api/agenda/agenda2/meetings/3pers/datetimes/, Josué ?

Frédéric Péters a écrit :

À creuser, c'est vraiment totalement un bug spécifique au fonctionnement interne d'IntervalTree, même du côté de leur suivi de bugs pour reproduire ils n'ont rien de simple, juste ce fichier monstrueux : https://github.com/chaimleib/intervaltree/files/694995/issue41a_test.txt Autant éviter une chose pareille ici. (i.e. ce n'est pas juste "retirer un intervalle qui n'existe pas")

Cela étant, sur des idées d'origine "simple" du soucis, j'ajoute un test sur un changement dans les heures d'ouverture et la vérification qu'une réservation qui désormais couvre la fin et le début d'un créneau bloque bien les deux.

Benjamin, concernant ton code, à regarder pour la couverture des tests je me rends compte qu'une part importante n'est pas utilisée du tout par Chrono (ce code vient d'un moment où c'était utile, ou c'était pour in fine à finir par construire un test de comparaison de perf avec intervaltree, ou ?).

Un peu tout ça, au départ je voulais juste m'amuser à voir si je pouvais refaire la même chose qu'intervaltree sachant que je savais que nos besoin étaient bien moindre et que donc un algorithme plus simple était possible (en gros c'est 10x plus rapide qu'intervaltree, bien sûr je ne couvre pas tout le spectre ce qu'intervaltree fournit, ou éventuellement pas avec les mêmes performances).

Bref j'ai réduit à la portion utile à Chrono; ce ticket ne se perdra de toute façon pas, s'il y a des éléments à ajouter plus tard, on les retrouvera.

Pas de souci ça me va, je crois qu'à part remove_overlap pas grand chose ne sert, par contre au passage il me semble que pas mal d'optimisation sont encore possible (je ne sais pas si le listing des dates disponibles est un truc qui au niveau performance pose souci donc je n'ai rien fait), notamment il me semble que get_all_slots() pourrait directement recevoir l'intervalle final au lieu que ce soit refiltré ensuite, on devrait pouvoir requêter plus facilement les évènements à retirer.

J'attache déjà le résultat mais je dois encore poser mes yeux attentivement sur le 0002, notamment réfléchir à la ligne :

[...]

Comme on demande à réserver à event.start_datetime, et que tous les slots retournés doivent avoir une durée correcte, je me demande ce que je rate en pensant que ça pourrait être :

[...]

(les tests passant correctement avec cette seconde version).

Ça me semble correct mais j'ai encore un peu du mal entre les durées, les slots, les event et les timeperiod, donc je te ferai confiance tu connais mieux le modèle de donnée que moi. Pour moi le code est encore assez moche et pourrait être simplifié. Y ayant repensé depuis je pense qu'on pourrait même simplifier jusqu'au point ou on a juste besoin d'une méthode intersect() et pas de passer par des IntervalSets dans get_all_slots() mais donc idée pour plus tard, idem si on met bien noir sur blanc que slot et event, exclusions etc.. ont des durée fixes dans une petite fourchette (15 minutes, 30 minutes, 1h, 2h, et c'est tou) )et donc sont posés sur une grille (heures divisés en quart d'heure) et pas arbitrairement sur l'axe des horaires on doit encore pouvoir simplifier le code et augmenter les performances (genre renvoyer tous les slots libre sur 6 mois bien bien vite), mais il me manque les définitions de base pour connaître les contraintes qui nous aideraient à simplifier les algos.

Dans mon code j'ai joué avec pytest-benchmark pas de souci sur le fait que ça dégage c'était juste pour me prouver que j'avais pas complètement planté le code par rapport à avant, mais ça pourrait être utile de connaître les perfs sur des exemples tordus (obtenir tous les slots sur 1 an pour des horaires compliqués et pleins d'exceptions) juste pour avoir une idée.

Pour se donner du mou sur la prod, j'ai "corrigé" en restaurant le code intervaltree d'origine mais en lui évitant son bug, via l'ajout de :

    for k, v in open_slots_by_desk.items():
        open_slots_by_desk[k] = v.copy()

Avant de lancer les remove_overlap.

Testé sur les différentes URL de mymeaux.fr où j'ai vu passer des traces, et d'autres encore.

Testé sur les différentes URL de mymeaux.fr où j'ai vu passer des traces, et d'autres encore.

Mais ce matin à nouveau une trace; vraisemblablement juste eu de la chance hier :/

Déplacé mes deux lignes :

    for k, v in open_slots_by_desk.items():
        open_slots_by_desk[k] = v.copy()

après la première série de remove_overlap et ça m'apparait tenir le coup.