Produits Entr'ouvert » Passerelle

Thomas Noël a écrit :
(Manie perso, mais j'ai du mal avec les fonctions déclarées après leur utilisation :) C'est-à-dire que je trouve que _call, _get_list et _get_value pourraient être au début de la classe)

Ma manie est de mettre les méthodes "privées" en fin de classe, c'est pour ça, mais va pour la tienne :)

Valentin Deniaud a écrit
Peut-être lié, on ne fait nulle part du 'type': ['string', 'null'], et ça fait mauvais ménage dans la présentation de l'endpoint en BO : soit en revenir au 'type': 'string' classique, soit introduire un patch préliminaire pour gérer ça.

Peut être qu'il y a une bonne manière de faire, mais je n'ai pas vu comment ne pas inclure un champ qui n'est pas défini côté WCS, lorsqu'on fait un appel WebService. Du coup si des champs ne sont pas définis, on a None, et mettre juste "string" provoquera une erreur côté passerelle puisque les données ne passeront pas la validation. S'il y a un moyen (simple) de ne pas envoyer les champs non définis, côté WCS, pourquoi pas, sinon je trouve ça bien de gérer le cas côté code. Tu parles d'un patch pour gérer la présentation en back office ?

Les mocks devraient être bêtes, juste servir une réponse à partir d'une URL, pas d'assert à l'intérieur.

Je ne suis pas d'accord, je ne vois aucun intérêt à mettre les asserts sur les données envoyées par Passerelle en dehors des mocks. De mon côté, je trouve que si pour une raison ou une autre un mock est appelé (ajout d'un test, cuisine interne aux endpoints passerelle...), les données envoyées par passerelle seront testées et c'est bien. Ca évite de dupliquer le code qui vérifie les données à chaque appel. Si c'est juste pour renvoyer une réponse, utiliser une fonction n'a pas plus d'intérêt que d'utiliser mock.patch.

Si il y a des choses à vérifier sur la requête émise elle est accessible à posteriori depuis l'objet mock en utilisant le décorateur @remember_called.

remember_called est très peu utilisé dans le code, non documenté (le deuxième résultat dans google à "remember_called httmock" c'est le code Entr'ouvert). Et je trouve ça très lourd comparé à juste tester la requête dans le mock, comme évoqué plus haut.

Je trouve que les tests dans test_api_entreprise.py sont biens.

En effet, j'ai copié du coup :)

Je trouve que test_create_request_errors est vraiment dur à lire, trop de niveaux d'indentation, trop de trucs qui s'appellent les uns les autres.

C'est relativement subjectif : il y a deux niveaux d'intentations, deux méthodes, aucun appel récursif : il y a pas mal de code qui risque d'être dur à lire à ce tarif. Si on parle en terme de complexité cyclomatique, utiliser une boucle for est même plus complexe que l'appel à une fonction imbriquée. Mais je comprends que ça soit un peu lourdingue, je ferais attention à ne pas abuser des fonctions imbriquées. J'ai aussi utilisé mock.patch, pour enlever un appel, c'est plus léger.

J'ai utilisé pytest.mark.parametrize. Benjamin, si tu as un avis sur la question ci-dessus, ça nous intéresse.

Valentin Deniaud a écrit :

Corentin Séchet a écrit :

je n'ai pas vu comment ne pas inclure un champ qui n'est pas défini côté WCS, lorsqu'on fait un appel WebService. Du coup si des champs ne sont pas définis, on a None, et mettre juste "string" provoquera une erreur côté passerelle puisque les données ne passeront pas la validation. S'il y a un moyen (simple) de ne pas envoyer les champs non définis, côté WCS, pourquoi pas

Je ne sais pas, interrogation à faire remonter à plus compétent que moi sur la question (Benjamin ?), ça m'a juste paru étrange que les autres connecteurs n'aient jamais eu à spécifier ça.

C'est parce que tu fais tes tests avec des expressions python coté w.c.s., avec des gabarits tu n'as jamais None, toujours une chaîne vide.

• se restreindre à n'utiliser que des gabarits coté w.c.s., ce qui est de toute façon la politique officiel de la secte EO&Co,
• ne pas déclarer le champ dans le schéma,
• mettre ['string', 'null'] qui est fait ailleurs en pagaille dans les connecteurs plus complexe contrairement à ce que dit Valentin, voir par exemple toulouse_axel. Mais en moins bien, j'avais l'habitude de faire des {'oneOf': [{'type': 'string'}, {'type': 'null'}]}, je ne connaissais pas cet usage de 'type', c'est effectivement plus court pour les types de base, oneOf devient nécessaire quand on a des validations plus complexes à combiner (genre object/null avec properties, pas sûr qu'on puisse utiliser type:object,null).

À noter qu'on peut utiliser aussi des références¹ pour raccourcir (mais je ne sais pas si c'est bien géré dans notre code de rendu des schéma, mais c'est un truc qu'on voit souvent en Swagger, donc prendre ma remarque avec des pincettes), ou bêtement une variable python 'STRING_OR_NULL_SCHEMA'[2].

¹ https://json-schema.org/understanding-json-schema/structuring.html

² https://git.entrouvert.org/passerelle.git/tree/passerelle/contrib/rsa13/models.py#n35

Benjamin Dauvergne a écrit :

• se restreindre à n'utiliser que des gabarits coté w.c.s., ce qui est de toute façon la politique officiel de la secte EO&Co,
• mettre ['string', 'null']

Faire l'un ou l'autre, mais le second nécessite un patch préliminaire d'adaptation du rendu des schémas comme noté plus haut.

Go.

commit 2b14a8d6c61512bf742ab3f25c13d3472735e19f
Author: Corentin Séchet <csechet@entrouvert.com>
Date:   Wed Jun 1 15:12:51 2022 +0200

    signal_arretes: create signal arretes connector (#65822)