Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Song files management (.sg) #19

Open
jmclem opened this issue Jan 28, 2014 · 72 comments
Open

Song files management (.sg) #19

jmclem opened this issue Jan 28, 2014 · 72 comments
Assignees
Milestone

Comments

@jmclem
Copy link
Contributor

jmclem commented Jan 28, 2014

I begin with management of song files. I foresee to first address management on the server, with following functionality/aspects :

  • passthrough of file content between interface and vcs:
    • file content is not stored into db and simply passed through to or from web frontend (FE)
    • no compilation is done within app backend (BE) (at first, maybe it comes later)
  • when saved from FE, BE commits a new version
    • maybe not immediately; time lapse (10 minutes without new save for example) or user switch could trigger commiting a new version
    • checking out could be used as locking mechanism to prevent parallel modifications. Or maybe a field in DB, don't know yet

Management in FE (browser) will come afterwards (displaying, editing).

Has someone other views / ideas ?

@ghost ghost assigned jmclem Jan 28, 2014
@Luthaf
Copy link
Contributor

Luthaf commented Jan 28, 2014

checking out could be used as locking mechanism to prevent parallel modifications. Or maybe a field in DB, don't know yet

Déolé, je n'arrrive pas à m'exprimer en anglais (vivement mon stage en angleterre ...)

On peut juste vérifier si personne d'autre n'a fait de modifs entre-temps. Pour ça, on peut utiliser l'étape de validation des formulaires (surchager la méthode validate()), et vérifier si le commit utilisé comme base de départ par l'utilisateur est identique au commit actuel. Dans le cas contraire, on renvoie à l'utilisateur un message comme quoi il y a eu des modification, et on l'invite à modifier les siennes.

no compilation is done within app backend (BE) (at first, maybe it comes later)

Je ne comprend pas cette phrase =(

Sinon, moi je l'aurai fait dans l'autre sens : d'abord la partie affichage, ensuite la partie sauvegarde et commit. Mais pourquoi pas dans ce sens =)

@jmclem
Copy link
Contributor Author

jmclem commented Jan 28, 2014

Honnêtement, je préfèrerais pour l'instant un lock strict (si quelqu'un a entamé des modifs, les autres attendent). J'aimerais pas que je fasse des modifs et que sur mon 'save' on me dise qu'il faut recommencer parce que quelqu'un d'autre a modifié entre temps, et je ne pense pas que le commun des guitaristes de feu de camp sache s'y retrouver dans un merge à trois voies ;) donc je n'envisage pas cette solution pour régler les conflits ...

no compilation is done within app backend (BE) (at first, maybe it comes later)

"le BE (partie de l'app sur le serveur) ne fait pas de compilation/transformation (au moins au début, peut-être plus tard)"

Sinon, moi je l'aurai fait dans l'autre sens : d'abord la partie affichage, ensuite la partie sauvegarde et commit. Mais pourquoi pas dans ce sens =)

Ça, c'est mon penchant naturel ;) de préferer faire fonctionner les services avant leur présentation à l'utilisateur. Tu l'auras sans doute perçu, je suis plutôt BE que FE.

@Luthaf
Copy link
Contributor

Luthaf commented Jan 28, 2014

Honnêtement, je préfèrerais pour l'instant un lock strict (si quelqu'un a entamé des modifs, les autres attendent). J'aimerais pas que je fasse des modifs et que sur mon 'save' on me dise qu'il faut recommencer parce que quelqu'un d'autre a modifié entre temps, et je ne pense pas que le commun des guitaristes de feu de camp sache s'y retrouver dans un merge à trois voies ;) donc je n'envisage pas cette solution pour régler les conflits ...

À ce moment, c'est la vue Edit qu'il faut bloquer, d'une manière ou d'une autre. Peut-être avec un décorateur ?

@Luthaf
Copy link
Contributor

Luthaf commented Jan 29, 2014

Question, finalement on utilise des VersionedFileField ou non pour les fichiers .sg ?

@jmclem
Copy link
Contributor Author

jmclem commented Jan 29, 2014

Je suis en train d'y jeter un oeil; je vois un problème : ce module crée ses propres noms de fichiers, tous dans un répertoire (cf https://github.com/Yaco-Sistemas/django-vff/blob/master/vff/field.py#L57 et https://github.com/Yaco-Sistemas/django-vff/blob/master/vff/git_backend.py#L97 ).
3 possibilités :

  • on modifie VFF pour pouvoir choisir les noms de fichiers - ça semble pas trop compliqué
  • on prend autre chose que VFF ou on le fait nous même - c'est peut-être plus casse pieds
  • on accepte le système de fichiers de VFF - mais alors fini le travail sur les fichiers et les liens avec patacrep
    Je penche pour la première solution, qu'en dites vous ?

@Luthaf
Copy link
Contributor

Luthaf commented Jan 29, 2014

Je suis pour la première solution aussi : ils semble qu'ils utilisent des noms de fichiers étranges pour garantir l'unicité des fichiers, ce dont on a moins besoin pour notre part (les noms étrange j'entends !)

@jmclem
Copy link
Contributor Author

jmclem commented Jan 31, 2014

Juste un petit point sur l'état des choses:

J'ai mené quelques réflexions et recherches concernant la modification des fichiers d'une part et l'ajout dans le VCS (séquence add-commit). Dans les deux cas, on a des risques de corruption suite à des accès parallèles (multi-thread et/ou multi-process).

J'envisage deux solutions:

  • un lock (fichier ou bdd)
  • des opérations clone/merge

J'ai moins de temps en ce moment, donc ça avance moins vite, mais je n'oublie pas ;)

Je peux rapidement programmer l'accès en lecture pour affichage si l'un de vous en a besoin; dites moi !

@oliverpool
Copy link
Contributor

Je pense qu'il est plus simple dans un premier temps de faire un lock

(les partiels approchant, mon temps disponible va fortement est réduit les prochains jours...)

@Luthaf
Copy link
Contributor

Luthaf commented Jan 31, 2014

Je pense qu'il est plus simple dans un premier temps de faire un lock

Ça me va aussi =)

(les partiels approchant, mon temps disponible va fortement est réduit les prochains jours...)

Moi je viens de finir, et je vais avoir une semaine de vacances avant de reprendre, donc ça va être l'inverse ^^

Par contre, en lien avec le stokage, je me demande comment sera gérée la position des fichiers : avec une fonction du style get_path(id=song_id) ? Ça influe du coup sur la génération des fichiers *.sb.

@oliverpool
Copy link
Contributor

un champ en BDD ?

@jmclem
Copy link
Contributor Author

jmclem commented Jan 31, 2014

Oui, un champ en bdd est une des idées. Ou alors utiliser une des fonctions de gitpython BlockingLockFile ou LockFile.

J'ai vaguement commencé avec une table en bdd, puis me suis décidé à poser la question sur SO qui ont apporté des avis plutôt négatifs sur les locks en général .

Je pense que cette réticence aux locks est à modérer dans notre cas par le fait que nous n'attendons pas souvent des éditions d'un fichier par plusieurs utilisateurs en parallèle, c'est plutôt le cas d'exception - dont il faut se protéger toutefois.

@Luthaf
Copy link
Contributor

Luthaf commented Jan 31, 2014

Je suis plutôt pour les fonctions de gitpython pour ma part -> le lock n'est ainsi lié qu'à une instance et pas à une entrée dans la BDD.

Concernant les réponses sur SO, je suis assez d'accord avec le dernier commentaire : ceux qui utilisent git n'aiment pas les verrous ! Mais dans notre cas c'est en effet moins gênant, les éditions successives seront plutôt des corrections que des réécritures complètes.

@oliverpool
Copy link
Contributor

légère imcompréhension : le champ en BDD était en réponse à "comment sera gérée la position des fichiers?"

@jmclem
Copy link
Contributor Author

jmclem commented Feb 1, 2014

Pour la position des fichiers : n'est ce pas pour l'instant le champ file_path dans le modèle GitFile ? Ou pensez vous à autre chose ?
Je penche toutefois pour remplacer le stockage du chemin dans système de fichiers par l'object id de git; ça compense les éventuels renommages ou déplacements des fichiers, et il y aurait une fonction "get_path" qui retournerait le chemin pour le workspace.

@Luthaf
Copy link
Contributor

Luthaf commented Feb 1, 2014

Pour la position des fichiers : n'est ce pas pour l'instant le champ file_path dans le modèle GitFile ? Ou pensez vous à autre chose ?

C'est ça pour l'instant, mais je me demandais ce que ça serait plus tard ^^ Je n'en ai pas encore besoin, mais ça sera utile pour la génération des fichiers .sb, dernière étape avant la compilation des carnets !

@jmclem
Copy link
Contributor Author

jmclem commented Feb 4, 2014

J'ai des interrogations... qui cherchent des réponses dans les 'use cases'. Commençons avec le point suivant:

  • Le 1er janvier, Antoine crée un carnet qui référence 10 chants, dont le chant chant1. Il génère son carnet et diffuse l'impression à sa troupe.
  • Le 2 février, Bénédicte modifie le chant1 pour corriger quelques accords.
  • Le 3 mars, Antoine a besoin de nouvelles impressions de son carnet. Il revient sur le site.

Que se voit-il alors présenter pour chant1 :

  1. l'ancienne version, avec un gentil message "Certains chants de votre carnet ont été mis à jour; voulez vous utiliser les nouvelles versions ou garder les anciennes?"
  2. ou la nouvelle version, sans lui laisser le choix? Au risque de changer son nombre de pages ou je ne sais quoi.

@Luthaf
Copy link
Contributor

Luthaf commented Feb 4, 2014

Moi je suis pour le message 1 =)

Du coup ça nécessite de stocker le commit en plus dans le carnet de chant, et de faire une comparaison. La question suivante est effectue-t-on cette comparaison lors de l'affichage du champ ou lors de la compilation du carnet ?

@oliverpool
Copy link
Contributor

Aussi pour le 1, mais je pense plutôt à une autre question:
est-ce qu'on doit recompiler le carnet à chaque fois ?
Je pense qu'il est plus judicieux de garder le pdf et de "forcer" l'utilisateur à demander une recompilation en lui précisant que cela écrasera la version précédente (qu'il ferait bien de sauvegarder...)

@Luthaf
Copy link
Contributor

Luthaf commented Feb 4, 2014

Pour la compilation, je pensais plutôt à une mise en cache : la dernière version PDF du carnet est mise en cache pendant 1 ou 2 semaine, puis supprimée du serveur. Mais il faut aussi discuter de ce point, en effet =)

@jmclem
Copy link
Contributor Author

jmclem commented Feb 5, 2014

Moi je suis pour le message 1 =)
Aussi pour le 1

moi aussi, on est tous d'accord 👍

PDF du carnet est mise en cache pendant 1 ou 2 semaine

C'est ce que j'aurais envisagé aussi.

La question suivante est effectue-t-on cette comparaison lors de l'affichage du champ ou lors de la compilation du carnet ?

Lorsque l'utilisateur ré-ouvre son carnet, on pourrait afficher

  • un message dans la page (c'est à dire pas un pop up) "ce carnet contient des chants mis à jours"
  • dans la liste des chants, une icône signalant la possibilité de mettre à jour.

Lorsque l'utilisateur clique sur la mise à jour d'un ou de tous le chants, deux possibilités:

  • on met à jour direct
  • on lui affiche un diff et il peut choisir

Je serais pour la mise à jour directe, mais il faut alors se mettre d'accord sur ce qu'est une mise à jour : une correction d'erreurs. Si une mise à jour pourrait être par exemple un changement de tonalité, je suis pour afficher le diff et laisser choisir.

@oliverpool
Copy link
Contributor

En bonus on pourrait proposer d'envoyer le carnet par mail (sorte de backup déporté)

Concernant la gestion des différentes versions (en fonction des commits), il faut d'abord un gros travail sur le moteur de rendu! (et en fonction de son implémentation, gérer celle du site web)

@Luthaf
Copy link
Contributor

Luthaf commented Feb 5, 2014

Concernant la gestion des différentes versions (en fonction des commits), il faut d'abord un gros travail sur le moteur de rendu! (et en fonction de son implémentation, gérer celle du site web)

Je pensais utiliser deux chemins d'inclusion : Soit le chant est à la dernière version et on met \input{SONG_LIBRARY/song.sg}; soit ce n'est pas la dernière version, et dans ce cas on récupère la dernière version dans un répertoire temporaire et on fait \input{TEMP/USER/song.sg}.

Je serais pour la mise à jour directe, mais il faut alors se mettre d'accord sur ce qu'est une mise à jour : une correction d'erreurs. Si une mise à jour pourrait être par exemple un changement de tonalité, je suis pour afficher le diff et laisser choisir.

À ce moment là, il faut soit stocker le type de mise à jour, soit choisir pour tous les chants. Concernant le changement de tonalité, il n'est pas nécessaire de mettre à jour le chant pour l'effectuer : le package song propose une commande \transpose{n}

@jmclem
Copy link
Contributor Author

jmclem commented Feb 6, 2014

Je pensais utiliser deux chemins d'inclusion : Soit le chant est à la dernière version et on met \input{SONG_LIBRARY/song.sg}; soit ce n'est pas la dernière version, et dans ce cas on récupère la dernière version dans un répertoire temporaire et on fait \input{TEMP/USER/song.sg}.

oui, on est ok

La tonalité, c'était juste un exemple de modification du contenu. Ça pourrait aussi être l'ajout d'un couplet.

@oliverpool
Copy link
Contributor

si un système de file d'attente est implémentée, le dossier TEMP peut être indépendant de l'utilisateur
Du coup il faudrait un "pré-processeur" de la liste des chants qui considère la révision voulue et duplique les chants si nécéssaire <= d'ailleurs si un chant est modifié pendant (ou juste avant) la compilation, ca donne quoi ?? On va finir par forcer la duplication pour éviter des conflits de ce genre...

@Luthaf
Copy link
Contributor

Luthaf commented Feb 6, 2014

Oui, en effet ...

Du coup, avec le préprocesseur, on peut récupérer dans le dossier temporaire l'ensemble des fichiers .sg nécessaires, avec toutes les versions ; et faire des \input{TEMP/song-<sha1>.sg}.

@jmclem
Copy link
Contributor Author

jmclem commented Feb 10, 2014

Aujourd'hui, on a ça pour la base de fichiers actuellement connus (réalisé avec AsciiFlow) :

   +---------+             1 +-------------+
   | Song    +-------------->| GitFile     |
   |---------+               |-------------|
   |         |               |object_hash  |
   |         |               |commit_hash  |
   |         |               |file_path    |
   |         |               |             |
   |         |               |             |
   +---------+               +-------------+

Il va falloir en plus stocker une information similaire pour les ItemsInSongbook, quelquechose comme :

   +---------+           +----------+         * +--------------+
   |Songbook +---------->| Items    +---------->|FileInSongbook|
   |---------|           |----------|           |--------------|
   |         |           |          |           |object_hash   |
   |         |           |          |           |commit_hash   |
   |         |           |          |           |file_path     |
   |         |           |          |           |              |
   +---------+           +----------+           |              |
                                                |              |
                                                |              |
                                                +--------------+

Pour la génération, seul le object_hash est nécessaire et permet d'écrire le contenu du fichier dans un cache.

C'est pour trouver les bonnes réponses aux questions suivantes que c'est plus galère :

  • le fichier référencé par un carnet a-t-il une nouvelle version ?
  • a-t-il été renommé, et où se trouve-t-il dans la révision actuelle ?

C'est pour cette raison que commit_hash et file_path sont aussi stockés actuellement.

Même si FileInSongbook et GitFile ont aujourd'hui les mêmes attributs, je les vois conceptuellement différents et hésite à les mettre dans un même modèle.

  • Ils évoluent à des moments différents: FileInSongbook lorsque le book est modifié (add/remove), et GitFile lorsque la db des chants est mise à jour (create/update) (plus tard aussi lorsqu'in chant est modifié par un utilisateur).
  • Ils ont des liens et cardinalités différentes aussi : un GitFile pour un Song; un ou plusieurs FileInSongbook pour chaque book (via itemsinsongbook).

Comme à la création du book les informations sont prises dans le GitFile, il contient le object_hash.

Qu'est ce que ça vous inspire ?

@jmclem
Copy link
Contributor Author

jmclem commented Feb 10, 2014

Un autre "problème" : j'ouvre mon carnet d'il y a un an qui contenait "la juman de micho". Une bonne âme a corrigé le titre entre temps. Mon carnet référence donc une version ancienne du fichier. Dans la table "Song", il y a maintenant "La jument de Michao". Le titre "la juman de micho" n'est plus accessible dans notre db.

Possibilités pour la gestion de carnet avec des versions anciennes de chants :

  • on affiche les infos mises à jour (ce qui ne marche plus si le chant a été supprimé)
  • avec chaque chant d'un carnet, je cache aussi song et artist
  • quand je vais chercher une version plus ancienne d'un fichier, je parse en temps réel le fichier pour en extraire les infos
  • on balance la gestion de version par dessus bord et on ne travaille qu'avec la dernière version d'un chant. Si le chant n'existe plus, il disparaît du carnet. Si quelqu'un a renommé/déplacé un fichier, il disparaît, à l'utilisateur de retourner inclure à nouveau le chant.

Stocker song et artist pour chaque chant dans un carnet pourrait donner ceci :

   +---------+           +----------+           +--------------+
   |Songbook +---------->| Items    +           |FileInSongbook|
   |---------|           |----------|           |--------------|
   |         |           |          |           |object_hash   |
   |         |           |          |           |commit_hash   |
   |         |           |          |           |file_path     |
   |         |           |          |           |              |
   +---------+           +---+------+           |              |
                             |                  |              |
                             |                  |              |
                             |*                 +--------------+
                             v                       ^
                         +--------+                  |1
                         |SongInSb+------------------+
                         |--------| *
                         |title   |
                         |        |              +----------+
                         |        +------------->|ArtistInSb|
                         |        | *          1 |----------|
                         +--------+              |name      |
                                                 |          |
                                                 |          |
                                                 +----------+

Qu'est ce que ça vous inspire ?

@Luthaf
Copy link
Contributor

Luthaf commented Feb 10, 2014

Pour ma part, je voyais plutôt l'ensemble comme ça :

     +-----------+       +-------------------+
     | Songbook  +-------> ItemsInSongbook   |
     |-----------|       |-------------------|     +---------+  +---------------+
     |           |       | rank              |     | Song    +-->  GitFile      |
     |           |       | item = GFK  +---------->|---------|  |---------------|
     |           |       |                   |     |         |  | commit_hash   |
     |           |       | object_hash       |     |         |  | object_hash   |
     |           |       |                   |     |         |  | filepath      |
     |           |       |                   |     +---------+  +---------------+
     |           |       |                   |
     +-----------+       |                   |
                         +-------------------+

Le champ object_hash pouvant être nul si le champ item n'est pas une chanson. Avec ce modèle, voici mes réponses aux question

le fichier référencé par un carnet a-t-il une nouvelle version ?

Je croit qu'il suffit de comparer la valeur de object_hash dans la table ItemsInSongbook à sa valeur dans GitFile pour savoir si le chant a été modifié.

a-t-il été renommé, et où se trouve-t-il dans la révision actuelle ?

Où se trouve-t-il : l'ID du chant n'a pas changée, donc pour retrouver le chemin de l'item 65 de mon carnet, je vais chercher item(id=65).item.file.path qui me renvoie le chemin actuel.

Toutefois ce système ne permet pas de savoir si le fichier a été renommé sans aller chercher l'ancienne version du fichier .sg.

Le titre "la juman de micho" n'est plus accessible dans notre db.

Ce n'est pas grave, vu que l'on accède pas aux chants par leur titre.

on affiche les infos mises à jour (ce qui ne marche plus si le chant a été supprimé)

Tu parle de l'affichage web ? Je suis pour afficher les infos mises à jour, ou un message : "ce chant a été supprimé, vous pouvez toutefois continuer à l'utiliser dans ce carnet" dans le cas où il l'a été.

quand je vais chercher une version plus ancienne d'un fichier, je parse en temps réel le fichier pour en extraire les infos

Pour moi, on ne va chercher les versions anciennes que lors de la compilation, ou de la visualisation de l'historique.

On peut aussi se dire que sauf suppression, les utilisateurs voudront bien des dernières versions, et afficher une page de mise à jour avant d'accéder à un carnet ancien. Sur cette page on affiche les différences, et on permet la mise à jour. Il ne reste à aller parser pour affichage que les chants qui ne sont pas à leur dernière version.

@oliverpool
Copy link
Contributor

Si un chant est supprimé, l'artiste correspondant n'est pas nécessairement supprimé !

(et je pense qu'on va se pourrir la vie a essayer de gérer les chants supprimés)

@Luthaf
Copy link
Contributor

Luthaf commented Feb 14, 2014

(et je pense qu'on va se pourrir la vie a essayer de gérer les chants supprimés)

Ça sert principalement à permettre aux utilisateurs de retrouver leurs carnets tels qu'ils étaient 1 ou 2 ans avant.

Une autre solution serait de ne pas gérer les versions de chants et de simplement garder les fichiers PDF correspondant aux carnets indéfiniment. En limitant du coup le nombre de carnets par utilisateurs (5 ou 10)

Une autre solution encore, à laquelle je viens de penser serait de garder les infos (artist, title, commit) du chant dans un champ supplémentaire du modèle ItemsInSongbook, par exemple de type JSON Field. C'est la question "où met-on les informations ?" : dans une autre table ou dans un champ supplémentaire. Cette dernière solution est moins souple, mais doit permettre de recréer le carnet à l'identique : la seule information utile pour la création du PDF est après tout le commit : si on peut récupérer le fichier originel, on a alors accès à toutes les infos. Et pour l'affichage, on peut effectuer une comparaison du commit, et si ce dernier diffère, aller chercher l'ancien titre et l'ancien artiste.

@jmclem
Copy link
Contributor Author

jmclem commented Feb 14, 2014

Une autre solution serait de ne pas gérer les versions de chants et de simplement garder les fichiers PDF correspondant aux carnets indéfiniment. En limitant du coup le nombre de carnets par utilisateurs (5 ou 10)

Oui, c'est une possibilité. On peut même dire que l'utilisateur est responsable de conserver son PDF si il veut le réutiliser plus tard, ça nous évite la responsabilité du stockage (à part pour une certaine durée, le temps qu'ils le récupèrent).

Une autre solution encore, à laquelle je viens de penser serait de garder les infos (artist, title, commit) du chant dans un champ supplémentaire du modèle ItemsInSongbook, par exemple de type JSON Field.

Vu le temps que prennent les accès via l'ORM, c'est une solution que je trouve tout à fait viable. Le stockage en table serait plus 'correct', mais si les temps de traitement explosent, bof. Il y a certainement moyen d'optimiser - juste les requêtes SQL pour les quelques copies à faire ne devraient pas prendre beaucoup de temps, mais est-ce-que ça vaut le coup?
Je serais partant pour cette option.

la seule information utile pour la création du PDF est après tout le commit :

En fait, l'object hash est même suffisant.

@oliverpool
Copy link
Contributor

Oui, c'est une possibilité. On peut même dire que l'utilisateur est responsable de conserver son PDF si il veut le réutiliser plus tard, ça nous évite la responsabilité du stockage (à part pour une certaine durée, le temps qu'ils le récupèrent).

👍

En fait, l'object hash est même suffisant.

👍

Vu le temps que prennent les accès via l'ORM, c'est une solution que je trouve tout à fait viable

il y a moyen de faire des JOIN et autre dans la BDD (c'est même fait pour ça ...)

@Luthaf
Copy link
Contributor

Luthaf commented Feb 14, 2014

il y a moyen de faire des JOIN et autre dans la BDD (c'est même fait pour ça ...)

La question est : quels avantages y a-t-il a utiliser des tables supplémentaires, et quels avantages à utiliser deux champs (object_hash et other_informations ) dans la même table. Là mes connaissances ne me permettent pas de répondre, mais je serait pour rester simple. (@jmclem : si on ajoute des champs dans ItemsInSongbook, désolé pour toute le boulot que tu as déjà fait)

@jmclem
Copy link
Contributor Author

jmclem commented Feb 14, 2014

si on ajoute des champs dans ItemsInSongbook, désolé pour toute le boulot que tu as déjà fait

merci pour ta compassion :) Même si ça fait ch... de faire des trucs qu'on doit jeter, je reste objectif : si c'est mieux, on fait comme ça. Quelquefois, faut essayer pour décider.

@oliverpool

il y a moyen de faire des JOIN et autre dans la BDD (c'est même fait pour ça ...)

je ne suis pas sûr de ce que tu veux dire. Je ne suis pas expert en django, mais il me semble que l'ORM fonctionne en lazy, et qu'il met des join là où il en faut.
Si on veut faire mieux en terme de perfs, il va falloir chercher. Je n'ai pas encore pu faire tourner de profiler, mais je serais curieux de savoir combien de temps est passé dans l'exécution du code et combien dans les requêtes. D'autre part, comment cela évolue-t-il avec un vrai serveur ? (je fonctionne avec manage.py runserver)

Perso, si on peut éviter le SQL direct, je préfère, à cause d'une part du traitement des risques d'injection et d'autre part du découplage des classes de modèles qui oblige à mettre à jour les requêtes à chaque modif des modèles.

@oliverpool
Copy link
Contributor

Je suis bien incapable de savoir le fonctionnement de l'ORM de django...

@jmclem si je comprends bien tu as implémenté le schéma d'un de tes messages précédents ?

   +---------+           +----------+           +--------------+
   |Songbook +---------->| Items    +           |FileInSongbook|
   |---------|           |----------|           |--------------|
   |         |           |          |           |object_hash   |
   |         |           |          |           |commit_hash   |
   |         |           |          |           |file_path     |
   |         |           |          |           |              |
   +---------+           +---+------+           |              |
                             |                  |              |
                             |                  |              |
                             |*                 +--------------+
                             v                       ^
   +-------+ 0..1      * +--------+                  |1
   | Song  |<------------+SongInSb+------------------+
   |-------|             |--------| *
   |title  |             |title   |
   |       |             |        |              +----------+
   |       |             |        +------------->|ArtistInSb|
   |       |             |        | *          1 |----------|
   +-------+             +--------+              |name      |
                                                 |          |
                                                 |          |
                                                 +----------+

(à propos duquel je suis encore dubitatif concernant la pertinence de ArtistInSb et SongInSb...à part pour la suppression dont nous n'avons pas vraiment discuté...)

@jmclem
Copy link
Contributor Author

jmclem commented Feb 14, 2014

si je comprends bien tu as implémenté le schéma d'un de tes messages précédents ?

oui

à part pour la suppression dont nous n'avons pas vraiment discuté

ce n'est pas faute d'avoir essayé...

@jmclem
Copy link
Contributor Author

jmclem commented Feb 14, 2014

Concernant les perfs : j'aimerais comprendre où le temps est perdu. Je vais essayer de profiler et de voir si on peut optimiser le truc. Si ça ne marche pas, je reconvertis avec le stockage en JSON dans ItemsInSongbook.

@oliverpool
Copy link
Contributor

ce n'est pas faute d'avoir essayé...

Il y a bien eu quelques messages, mais aucune décision clairement prise à ma connaissance ? (#35)

@Luthaf
Copy link
Contributor

Luthaf commented Feb 14, 2014

Je suis bien incapable de savoir le fonctionnement de l'ORM de django...

Il fonctionne effectivement en lazy. Tout est là : https://docs.djangoproject.com/en/dev/ref/models/querysets/

D'autre part, comment cela évolue-t-il avec un vrai serveur ?

Je ne sais pas. Il peut fonctionner en CGI avec apache si j'ai tout bien compris. Je peut tenter de faire des test là dessus si besoin.

Sinon pour les requêtes, elles sont donnée en détail par django-toolbar.

@jmclem
Copy link
Contributor Author

jmclem commented Feb 15, 2014

J'ai fait tourner un profiler chez moi; le temps était passé dans les commits :

   Ordered by: internal time, call count
   List reduced from 557 to 20 due to restriction <20>

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
      945   48.214    0.051   48.217    0.051 /usr/local/lib/python2.7/dist-packages/django/db/backends/__init__.py:133(_commit)
     1110    0.775    0.001    0.810    0.001 /usr/lib/python2.7/dist-packages/MySQLdb/cursors.py:277(_do_query)
     1891    0.196    0.000    0.196    0.000 /usr/local/lib/python2.7/dist-packages/django/db/backends/mysql/base.py:459(_set_autocommit)

Après recherches, j'ai compris que la cause en est que par défaut django est en autocommit : chaque appel db est suivi d'un commit (voir la doc).
C'est d'une part consommateur de temps et d'autre part présente le risque de laisser une db dans un état intermédiaire en cas d'exception : une partie des données sont committées, pas le reste.
La solution est d'ajouter des transactions. Pour l'instant, j'ai fait un commit qui ajoute 'ATOMIC_REQUESTS': True à la configuration de la db. Ainsi, tous les appels via les vues sont automatiquement encapsulés dans une transaction.
Il peut y avoir des cas où ce n'est pas souhaité, on agira à ce moment là (il existe un decorator pour inhiber la transaction systématique).
Avec çà, on retrouve des temps normaux :

   Ordered by: internal time, call count
   List reduced from 980 to 20 due to restriction <20>

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
     1802    0.653    0.000    0.708    0.000 /usr/lib/python2.7/dist-packages/MySQLdb/cursors.py:277(_do_query)
     4964    0.155    0.000    0.387    0.000 /usr/local/lib/python2.7/dist-packages/django/db/models/sql/query.py:213(clone)
     1578    0.126    0.000    0.589    0.000 /usr/local/lib/python2.7/dist-packages/django/db/models/sql/compiler.py:923(as_sql)

@oliverpool
Copy link
Contributor

Effectivement la différence est visible!
pour info, c'est quelle page que tu as testé ? (pour savoir la durée de l'ajout des chansons de "Naheulbeuk")

@Luthaf
Copy link
Contributor

Luthaf commented Feb 17, 2014

Alors pour la partie synchronisation du dépôt avec la bdd, on peut utiliser ça (http://developer.github.com/webhooks/) pour savoir quand un push a été fait sur le dépôt.

@jmclem
Copy link
Contributor Author

jmclem commented Feb 18, 2014

Décidément, GitHub est plein de ressources...

Plus généralement, je me pose la question de qui va ordonner la mise à jour. À terme, je verrais bien une notification via le site web ou email d'un admin lorsque des mises à jour sont disponibles. L'admin a alors la possibilité de déclencher manuellement la mise à jour.

Les hooks mentionnés par @Luthaf pourraient avoir leur place dans la notification.

@Luthaf
Copy link
Contributor

Luthaf commented Feb 18, 2014

L'admin a alors la possibilité de déclencher manuellement la mise à jour.

Pourquoi pas =) Par exemple en ajoutant un lien dans la zone d'administration des chants pour synchroniser la base de donnée avec le dépôt.

Les push depuis le dépôt du site web vers github auraient lieu quand ? Tous les jours s'il y a eu des modifications ?

@jmclem
Copy link
Contributor Author

jmclem commented Feb 18, 2014

Les push depuis le dépôt du site web vers github auraient lieu quand ?

Bonne question. Toutes les installs n'auront pas nécessairement des repos avec github comme origin, et certains peuvent vouloir faire tourner ça en privé. Je peux aussi avoir sur une repo github comme origin, mais pas le droit de pusher dessus.

D'autre part, à partir du moment où nous modifions les chants sur l'install, on s'expose à des gestions plus complexes: pull comme push peuvent conduire à des conflits de merges. Tant que songbook-web ne modifie pas la repo, on est en push simple, et nous n'avons pas ces conflits.

@oliverpool
Copy link
Contributor

ca pourrait peut-être nécessiter une issue complète, genre "Gestion des versions"


enfin, on est raisonnable et on attend, quoi ;)

on risque d'ajouter pas mal de fonctions qui risquent de ne pas être utilisées si on attend pas assez...

Une version hyper-light pourrait être simplement:
le site web héberge son propre repo .git (par exemple url/api/songs.git)

Le moyen principal de mise à jour est via le site web. Pour les trucs plus complexes, on laisse les utilisateurs expérimentés cloner ce répertoire et faire leurs merge comme ils veulent (avec d'autres répertoires par exemple)...
L'utilisateur qui utilisera notre site ne connait déjà pas le mot commit, alors lui parler de merge est peut-être un peu ambitieux ;-)

@Luthaf
Copy link
Contributor

Luthaf commented Feb 18, 2014

ca pourrait peut-être nécessiter une issue complète, genre "Gestion des versions"

Allons-y ! Et je pense que l'on peut fermer celle là, non ?

@jmclem
Copy link
Contributor Author

jmclem commented Feb 18, 2014

Tout à fait d'accord. Je propose quand même de garder cette issue ouverte tout de même jusqu'à ce que l'update depuis le site web soit possible (càd mise à jour de la repo, des tables song et artist).

@jmclem jmclem added this to the 0.1 milestone Feb 24, 2014
@Luthaf Luthaf modified the milestones: 0.2, 0.1 Apr 21, 2014
@Luthaf Luthaf modified the milestone: 0.2 Jun 4, 2014
@Luthaf Luthaf modified the milestones: 0.3, 0.2 Jun 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants