Produits Entr'ouvert » Passerelle

(Le lien dans la description pointe vers une page avec zéro information)

+ category = _('Business Process Connectors')
+ verbose_name = u"ActesWeb - Demande d'acte d'état civil"

Je serais pour avoir les différents connecteurs état civil dans leur catégorie propre, plutôt qu'inclure dans le nom du connecteur le fait que c'est de l'état civil.

+MATCHING = {

Il y a sans doute mieux comme nom. Et c'est utilisé à un seul endroit, pas besoin de taper ça de manière globale, je trouve.

+ demande_template_name = 'actesweb/demand.txt'

Sa présence à cet endroit me faisait penser que ça pouvait être un élément partagé avec les autres connecteurs mais non, et pareil, utilisé à un seul endroit il aurait plutôt sa place dans create_demand, pour ne pas allonger le jeu de piste.

+def process(payload):

Autre commentaire dans la série "pourquoi pas dans la classe ?"

+COMMENTAIRE={{application_comment}}

Il n'y a rien pour interdire des retours à la ligne ici.

Je suis un peu étonné de l'utilisation de default_storage... on est sûr qu'en construisant le chemin comme cela on aura jamais de doublon ?

+        filepath = os.path.join(self.basepath, application_time + '.DEM')

est-ce que ActesWeb s'attend à un nommage particulier ou lit tout ce qui finit par '.DEM' ?

+def get_test_base_dir(name):
+    return os.path.join(os.path.dirname(__file__), 'data', name)

Les fichiers temporaires on met ça dans un settings.MEDIA temporaire, via mkdtemp() et avec une fixture et on ne nettoye pas avec un teardown_module.

Si tu as une fixture utilisée par tous tes tests tu peux utiliser autouse=True:

import temptfile
import shutil

@pytest.fixture(autouse=True)
def media_dir(settings):
    settings.MEDIA = tempfile.mkdtemp()
    yield
    shutil.rmtree(settings.MEDIA, ignore_errors=True)

Je prends en compte vos commentaires puis je corrige dans la branche.

Réponses à Benjmain

Je suis un peu étonné de l'utilisation de default_storage... on est sûr qu'en construisant le chemin comme cela on aura jamais de doublon ?
[...]

Oui je me suis posé la question aussi mais j'ai préferé suivre leur spec. Le mieux d'avoir l'id de la demande au lieu de millisecondes.

est-ce que ActesWeb s'attend à un nommage particulier ou lit tout ce qui finit par '.DEM' ?

Oui l'application attend un nommage particuler : %Y-%m-%d_%H-%M-%S_%f.DEM

[...]

Les fichiers temporaires on met ça dans un settings.MEDIA temporaire, via mkdtemp() et avec une fixture et on ne nettoye pas avec un teardown_module.

Si tu as une fixture utilisée par tous tes tests tu peux utiliser autouse=True:
[...]

Ok

Branche mise à jour en prenant en compte vos commentaires.

Josué Kouka a écrit :

Je prends en compte vos commentaires puis je corrige dans la branche.

Réponses à Benjmain

Je suis un peu étonné de l'utilisation de default_storage... on est sûr qu'en construisant le chemin comme cela on aura jamais de doublon ?
[...]

Oui je me suis posé la question aussi mais j'ai préferé suivre leur spec. Le mieux d'avoir l'id de la demande au lieu de millisecondes.

est-ce que ActesWeb s'attend à un nommage particulier ou lit tout ce qui finit par '.DEM' ?

Tu ne suis pas la spéc: « @où AAAA est l’année de création du fichier» », application_time c'est un truc envoyé au WS, ce n'est pas du tout la date de création du fichier, je reconnais qu'avec des microsecondes c'est dur de faire des doublons mais bon je ne dépendrai pas sur ce coup là du client qui pourrait nous envoyer des trucs précis seulement à la seconde. Je trouverai plus clair de faire:

while True:
    filename =  django.utils.timezone.now().strftime('%Y-%m-%d_%H-%M-%S_%f.DEM')
    filepath os.path.join(base, filename)
    saved_name = default_storage.save(filepath, content)
    if filename == saved_name:
        break
    default_storage.delete(saved_name)

On sait comment l'application viendra lire les fichiers ? Pas de possibilité qu'elle vienne lire un fichier en train d'être écrit ? (voilà pourquoi je n'aime pas les classes storage de Django, si c'est juste pour manipuler des fichiers on est plus à l'aise en python pur, à faire des O_CREATE, fsync, des link/unlink, default_storage on lui demande simplement de nous trouver un répertoire à la rigueur).

Benjamin Dauvergne a écrit :

Josué Kouka a écrit :

Je prends en compte vos commentaires puis je corrige dans la branche.

Réponses à Benjmain

Je suis un peu étonné de l'utilisation de default_storage... on est sûr qu'en construisant le chemin comme cela on aura jamais de doublon ?
[...]

Oui je me suis posé la question aussi mais j'ai préferé suivre leur spec. Le mieux d'avoir l'id de la demande au lieu de millisecondes.

est-ce que ActesWeb s'attend à un nommage particulier ou lit tout ce qui finit par '.DEM' ?

Tu ne suis pas la spéc: « @où AAAA est l’année de création du fichier» », application_time c'est un truc envoyé au WS, ce n'est pas du tout la date de création du fichier, je reconnais qu'avec des microsecondes c'est dur de faire des doublons mais bon je ne dépendrai pas sur ce coup là du client qui pourrait nous envoyer des trucs précis seulement à la seconde.

Vrai, je ne sais pas pourquoi dans ma tete c'était la date de reception de la demande.
Je trouverai plus clair de faire:

[...]
avec un test de collision si par un immense hasard on créait deux demandes au même moment.

On sait comment l'application viendra lire les fichiers ? Pas de possibilité qu'elle vienne lire un fichier en train d'être écrit ?

Ce sera identique à ce qui a été fait dans les autres cas. On liste et deplace les fichiers dans un repertoire, on les traite, puis on les déplace dans un repertoire Traité.

Euh, qui liste ? qui déplace ? Un script shell lancé par CRON ? Ça fait just mv /var/lib/...media/actesweb/* /var/lib/sftp/acteweb/ ?

Dans ces cas ce serait mieux de faire un write -> actesweb/tmp/, fsync(), puis rename() vers actesweb/new/ et comme ça on peut dormir.

Benjamin Dauvergne a écrit :

Euh, qui liste ? qui déplace ? Un script shell lancé par CRON ? Ça fait just mv /var/lib/...media/actesweb/* /var/lib/sftp/acteweb/ ?

Un script du client, qui prend les demande et les met dans un dossier afin de les traiter.

Dans ces cas ce serait mieux de faire un write -> actesweb/tmp/, fsync(), puis rename() vers actesweb/new/ et comme ça on peut dormir.

Ok, j'ai mis à jour la branche avec les modifs.

Je ne comprends pas, ça ne dépend pas des patchs précédents de la branche ?

Le patch a été refait pour ne pas dependre de l'interface "état civil": http://git.entrouvert.org/passerelle.git/commit/?h=wip/aec_interface&id=5a60f547aa5b721b52faa2599105f8372bba39c5

Il fait référence à une dependance de migration non-existente, mais une fois corrigé, les tests passent.

Après discussion avec Brice il semble que les tentatives d'établir une interface générique fonctionnelle pour le connecteur d'état civil n'ont pas abouti: trop de spécificités pour les logiciels.

Je ne suis pas d'accord mais il faut alors aller noter ça dans #16768, et le rejeter.