geo2france / idg-qgis-plugin

Plugin QGIS pour la consultation des données des différentes Infrastructure de Données Géographiques en France
GNU General Public License v2.0
14 stars 4 forks source link

Téléchargement d'une IDG lors de validation des settings #78

Open bchartier opened 3 months ago

bchartier commented 3 months ago

Ce ticket est dérivé de #74

Dans le ticket cité ci-dessus, je relevais que ce n'était pas logique de traiter le téléchargement des fichiers de manières différentes dans dlg_settings.py et post_ui_init de plugin_main.py. Je me suis trompé sur ce point.

Un traitement particulier réalisé dans la méthode apply de la classe ConfigOptionsPage (dlg_settings.py) peut se justifier car l'utilisateur pourrait ajouter une nouvelle IDG. Aujourd'hui une validation de la modification des settings entraîne systématiquement le téléchargement de tous les projets qui ne sont pas masqués (appel de DownloadAllIdgFilesAsync).

À mon avis : Pour optimiser la chose il faudrait ne télécharger que les fichiers pour lesquels il y a eu une modification réalisée par l'utilisateur dans la fenêtre dlg_settings. Parce qu'aujourd'hui, on télécharge les fichiers de toutes les IDG non masquées même si on clique sur le bouton Ok sans rien modifier. Même chose si l'on modifie un paramètre général du plugin qui n'est pas spécifiquement associé à une IDG (par exemple debug_mode, download_files_at_startup).

Par exemple au lieu de surcharger les settings comme cela : https://github.com/geo2france/idg-qgis-plugin/blob/d4252ee377054c69f5d19053988ff9b2ab14651d/plugin/idg/gui/dlg_settings.py#L133-L145 on pourrait comparer les valeurs avant et après modif de settings.custom_idgs et settings.hidden_idgs pour identifier quels fichiers de quelles IDG doivent être téléchargés.

Pour savoir si un fichier doit être téléchargé il faudrait idéalement :

Un fichier absent qui doit être ajouté à l'explorateur doit être téléchargé. Un fichier déjà présent alors download_files_at_startup ne vaut pas True ne devrait pas être téléchargé.

@jbdesbas : qu'en penses-tu ?

jbdesbas commented 3 months ago

Je suis assez mitigé, je trouve que ca ajoute beaucoup de complexité et de risque de dysfonctionnement pour un gain limité. L'édition des paramètres du plugin reste une action assez rare (pas quotidienne en tout cas) : forcer un re-téléchargement complet des IDG non masquées en cas de changement de config offre au moins l'assurance d'une certaine cohérence entre les différents fichiers. C'est assez bourrin j'en conviens, mais on reste sur des volumes transférés relativement faibles : les QGZ utilisés ici font généralement moins de 500ko (les QGS sont plus lourd, mais peut-être à éviter ?)

bchartier commented 3 months ago

Je suis assez mitigé

Oui, c'est pour cela que j'en ai fait un ticket séparé. À traiter plus tard ou à abandonner.

on reste sur des volumes transférés relativement faibles

Le problème n'est pas tant le téléchargement que le refresh du provider qui s'en suit. La lecture du fichier QGZ par QGIS est très longue. Mon idée est que si l'on ne télécharge pas les fichiers on n'a pas à rafraichir le provider dans l'explorateur de QGIS. Cela serait plus confortable. [Maintenant, si on télécharge un peu moins on est un peu plus écoresponsable.]

jbdesbas commented 3 months ago

En effet, il y a peut-être une optimisation à rechercher du côté de la lecture du QGZ, par exemple en les traitant au fur et à mesure de leurs téléchargement, voir même en parallèle (actuellement on attends que tout soit télécharger, et ils sont traité un par un).

OK pour garder le ticket pour l'instant

bchartier commented 3 months ago

Ça marche on laisse murir car pas d'urgence, pas critique.