Project

General

Profile

Development #27412

pouvoir supprimer une visualisation depuis le listing

Added by Serghei Mihai about 1 year ago. Updated 13 days ago.

Status:
Solution déployée
Priority:
Normal
Target version:
-
Start date:
19 Oct 2018
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

Description

Très utile quand les données du formulaire derrière n'existent plus.
Accessoire, mais on peut aussi signaler quand les données n'existent plus, genre griser le titre de la visu.

0002-templates-allow-deleting-visualization-from-listing-.patch View (1.34 KB) Valentin Deniaud, 30 Oct 2019 11:22 AM

0001-views-grey-out-unavailable-visualizations-27412.patch View (5.79 KB) Valentin Deniaud, 30 Oct 2019 11:22 AM

0002-templates-allow-deleting-visualization-from-listing-.patch View (1.36 KB) Valentin Deniaud, 30 Oct 2019 12:27 PM

0001-views-grey-out-unavailable-visualizations-27412.patch View (5.88 KB) Valentin Deniaud, 30 Oct 2019 12:27 PM

0001-views-grey-out-unavailable-visualizations-27412.patch View (7.36 KB) Valentin Deniaud, 30 Oct 2019 03:46 PM

0002-views-grey-out-unavailable-visualizations-27412.patch View (7.24 KB) Valentin Deniaud, 20 Nov 2019 05:18 PM

0003-templates-allow-deleting-visualization-from-listing-.patch View (1.32 KB) Valentin Deniaud, 20 Nov 2019 05:18 PM

0001-utils-cache-warehouses-for-same-request-27412.patch View (1.03 KB) Valentin Deniaud, 20 Nov 2019 05:18 PM

0004-templates-allow-deleting-visualization-from-listing-.patch View (1.03 KB) Valentin Deniaud, 21 Nov 2019 11:58 AM

0003-views-grey-out-unavailable-visualizations-27412.patch View (7.12 KB) Valentin Deniaud, 21 Nov 2019 11:58 AM

0002-utils-cache-warehouses-27412.patch View (1.87 KB) Valentin Deniaud, 21 Nov 2019 11:58 AM

0001-utils-get-tenant-from-import-rather-than-request-274.patch View (5.67 KB) Valentin Deniaud, 21 Nov 2019 11:58 AM

0001-utils-be-more-efficient-at-caching-warehouses-27412.patch View (1.23 KB) Valentin Deniaud, 21 Nov 2019 05:02 PM

Associated revisions

Revision 7a5373c9 (diff)
Added by Valentin Deniaud 24 days ago

utils: get tenant from import rather than request (#27412)

Revision ba2c0c4f (diff)
Added by Valentin Deniaud 24 days ago

utils: cache warehouses (#27412)

Disk reads take a long long time.

Revision a0caedde (diff)
Added by Valentin Deniaud 24 days ago

views: grey out unavailable visualizations (#27412)

Revision 370a8e73 (diff)
Added by Valentin Deniaud 24 days ago

templates: allow deleting visualization from listing (#27412)

Revision 90a5bb49 (diff)
Added by Valentin Deniaud 20 days ago

Revert "utils: cache warehouses (#27412)"

This reverts commit ba2c0c4fabba4d3ff71dde36ede01c812bc73282.

History

#1 Updated by Valentin Deniaud about 2 months ago

  • Assignee set to Valentin Deniaud

#2 Updated by Valentin Deniaud about 2 months ago

J'ai fait les deux.
Le patch 2 n'est pas suffisant, je ne sais pas comment faire pour que le lien de suppression s'affiche sur la même ligne en restant dans les clous de gadjo. J'ai essayé de mettre les mêmes classes que les listes à boutons dans wcs, mais une partie du style que les fait marcher est dans admin.css, donc ça n'a pas fonctionné.

#3 Updated by Frédéric Péters about 2 months ago

le lien de suppression s'affiche sur la même ligne en restant dans les clous de gadjo

Tu peux regarder l'écran de configuration d'un agenda d'événements dans chrono, ça fait exactement ça.

#4 Updated by Benjamin Dauvergne about 2 months ago

Valentin Deniaud a écrit :

J'ai fait les deux.
Le patch 2 n'est pas suffisant, je ne sais pas comment faire pour que le lien de suppression s'affiche sur la même ligne en restant dans les clous de gadjo. J'ai essayé de mettre les mêmes classes que les listes à boutons dans wcs, mais une partie du style que les fait marcher est dans admin.css, donc ça n'a pas fonctionné.

Pourquoi une staticmethod au lieu de juste faire visualization.cube_exists dans le template ? Ça m'a l'air inutilement tordu (et puis ça pourrait s'appeler simplement .exists ou .valid ou .ok).

#5 Updated by Valentin Deniaud about 2 months ago

class="delete" à ajouter, c'est magique merci. J'en profite pour corriger un bug sur le patch 1, et renommer la méthode en exists. Manque éventuellement un test à ajouter.

Pourquoi une staticmethod au lieu de juste faire visualization.cube_exists dans le template ?

C'est utils.Visualisation qui a les méthodes qui nous intéressent, pas models.Visualisation qu'on a dans le template. On pourrait certainement ajouter la méthode exists au modèle, mais ça n'a pas l'air d'être trop dans l'esprit du code.

#6 Updated by Valentin Deniaud about 2 months ago

La même avec un test.

Je note que c'était pas très sympa comme expérience, tox -- -k test_missing_data ne fonctionne pas (tout plein de tests sont sélectionnés) et met très longtemps. La faute sûrement à pytest_generate_tests dans le même fichier...

#7 Updated by Valentin Deniaud 25 days ago

Valentin Deniaud a écrit :

tox -- -k test_missing_data [...] met très longtemps

Bon, c'était à cause de mon patch, j'ai un peu tâtonné pour comprendre pourquoi. Au début je soupçonnais la boucle for qui récupère un cube en itérant sur une liste dans schemas.py (du coup j'ai un patch pour changer ça si ça peut servir), mais en fait les coupables étaient les accès disques lors de la récupération des warehouses. J'ai bricolé un cache sur l'objet requête et hop, les builds reviennent de x heures à une durée normale.

#8 Updated by Benjamin Dauvergne 25 days ago

C'est toujours assez moche, je préfèrerai vraiment un {% if visualization.exists %} pour ce qui est de l'esprit du code si il peut être amélioré tant mieux (il y a effectivement beaucoup de chose dans utils.py si ça peut être progressivement remigrer sur le modèle ce n'est pas plus mal).

Pour tout le reste c'est bon.

#9 Updated by Valentin Deniaud 24 days ago

Oui mais il nous faut un accès à request, je ne connais pas de méthode pour m'en sortir avec un appel depuis le template (à part le stockage global de l'objet via un middleware mais a-t-on envie d'en arriver là). Sinon un template tag ?

#10 Updated by Benjamin Dauvergne 24 days ago

Valentin Deniaud a écrit :

Oui mais il nous faut un accès à request, je ne connais pas de méthode pour m'en sortir avec un appel depuis le template (à part le stockage global de l'objet via un middleware mais a-t-on envie d'en arriver là). Sinon un template tag ?

Le tenant est disponible globalement sur django.db.connection.tenant on peut passer par là plutôt que par request dans get_warehouse(), le cache peut-être fait globalement avec une durée courte (genre 30 secondes) par rapport au chemin, ce n'est pas un truc qui bouge.

#12 Updated by Benjamin Dauvergne 24 days ago

  • Status changed from Solution proposée to Solution validée

#13 Updated by Valentin Deniaud 24 days ago

  • Status changed from Solution validée to Résolu (à déployer)
commit 370a8e73e3c379ff1fbb3dcd884bc6b3d7c39925
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Wed Oct 30 10:56:27 2019 +0100

    templates: allow deleting visualization from listing (#27412)

commit a0caedde62d10d6880ca7ed9e4bc057af880eee6
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Tue Oct 29 17:46:21 2019 +0100

    views: grey out unavailable visualizations (#27412)

commit ba2c0c4fabba4d3ff71dde36ede01c812bc73282
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Thu Nov 21 11:54:21 2019 +0100

    utils: cache warehouses (#27412)

    Disk reads take a long long time.

commit 7a5373c91ed6ef5e1b6eca095a8cf0b28972d5ca
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Thu Nov 21 11:53:27 2019 +0100

    utils: get tenant from import rather than request (#27412)

#14 Updated by Valentin Deniaud 24 days ago

Mince j'avais pas vu mais le build jenkins est redevenu assez long (1h30 au lieu de 20min)... Rien à voir avec les 10h que ça prend sans cache, mais quand même bien problématique.

Pour mettre les choses à plat :
  1. Pour voir si les données existent, on doit aller les demander aux objets Warehouse
  2. La fonction get_warehouses fait des lectures sur le disque, c'est long, mais ça n'était pas grave car il y avait un seul appel au moment d'aller voir une visu
  3. Maintenant à l'affichage de la page d'accueil, on a autant d'appel à get_warehouses que de visualisations, c'est lent
  4. test_schema2.py lance 400 fois un test et affiche à chaque fois la page d'accueil, donc 400^2 appels à get_warehouses, c'est très lent
  5. Il faut donc brancher un système de cache pour cette fonction
  6. Première idée : cache comme un simple attribut sur la requête, ce qui réduit le nombre de vrais appels à 400 --> les tests retrouvent une durée d'exécution normal
  7. Sauf qu'on supprime l'objet request dans get_warehouses pour des raisons plus haut, donc recours au cache django, ce qui réduit le nombre de vrais appels à 2 (les deux schémas différents utilisés dans les tests).
    Et je n'avais pas pensé à vérifier mais en fait c'est un peu lent (0.07s à 0.03s sans cache, de 0.01 à 0.001 avec).

Je vais continuer d'investiguer, et finir par trouver une solution.

#15 Updated by Valentin Deniaud 24 days ago

En fait ce qui est lent c'est le pickle/unpickle.
Ma proposition de solution toute simple avec une variable globale. Le retour de get_warehouses n'est jamais modifié, facile à vérifier puisqu'il est toujours utilisé dans for x in get_warehouses(). Je garde quand même le cache pour gérer l'expiration, mais peut-être que c'est un moyen détourné et qu'il y a plus direct.

#16 Updated by Benjamin Dauvergne 23 days ago

Passe sur un autre ticket.

#17 Updated by Valentin Deniaud 20 days ago

  • Status changed from Solution proposée to Résolu (à déployer)

(retour dans le bon statut)

#18 Updated by Frédéric Péters 13 days ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF