Feat/cisu converter #239
Feat/cisu converter #239
Conversation
muffoltz
force-pushed
the
feat/cisu-converter
branch
from
9ff3e61 to
85f379f
Compare
muffoltz
force-pushed
the
feat/cisu-converter
branch
from
2fb62c5 to
67a59a9
Compare
|
romainfd
added
the
Review Ready
|
There was a problem hiding this comment.
C'est top ! Je m'interroge un peu sur comment on rend tout ça bien robuste (pas de perte de données, pas de plantage sur des champs manquants, ...). Est-ce que c'est juste versionner les outputs et s'assurer qu'ils sont OK ? Est-ce qu'on essaie des jeux de tests sur des messages générés avec un truc de ce genre https://github.com/json-schema-faker/json-schema-faker ? Ca va être le point le plus sensible (parce que le dispatcher en lui-même fait très peu de manips et est déjà bien rôdé) des échanges 15-NexSIS (qui sont nos échanges les plus sensibles) donc il faut qu'on soit le plus béton possible !
| new_alert = format_object(input_usecase_json.get('newAlert')) | ||
| add_object_to_initial_alert_notes(output_usecase_json, new_alert) | ||
|
|
||
| # - Deletions - must to be the last step |
There was a problem hiding this comment.
Pourquoi ? On s'appuie sur input_usecase_json pour construire le contenu
There was a problem hiding this comment.
tu trouves ça étrange ? comme on applique des règles à un message en entrée, ça me parait cohérent d'utiliser input_usecase_json, et ça permet de s'abstraire des modifications appliquées à output_usecase_json, t'en penses quoi ?
There was a problem hiding this comment.
Ah je pense qu'on s'est mal compris.! Mon commentaire porte sur la ligne 102, la ligne 99 me va très bien perso et je suis complètement d'accord que c'est mieux de faire sur input plutôt que de se baser sur output qui est en train d'être modifié !
romainfd
added
Last Changes
muffoltz
force-pushed
the
feat/cisu-converter
branch
from
f063b64 to
b5ad12f
Compare
|
muffoltz
force-pushed
the
feat/cisu-converter
branch
from
b5ad12f to
9b301d9
Compare
|
muffoltz
force-pushed
the
feat/cisu-converter
branch
from
9b301d9 to
74c7464
Compare
|
muffoltz
force-pushed
the
feat/cisu-converter
branch
from
74c7464 to
9713f1d
Compare
|
There was a problem hiding this comment.
Ca me semble top pour une première version de convertisseur ! On avisera lors du baptême du feu s'il y a des choses à renforcer / corriger et/ou plus de logs / error management à ajouter mais c'est vraiment super comme base pour avancer !
Et trop bien la refacto avec les fonctions is_field_completed et get_field_value et tous les tests dessus ! Bien plus clair et robuste !
Quelques petits retours et l'ajout des fichiers de tests de non régression à mettre après validation avec Daphné puis on pourra merger ça et le déployer !
|
|
||
| add_location_detail(output_use_case_json) | ||
|
|
||
| if is_field_completed(output_use_case_json,'$.initialAlert'): |
There was a problem hiding this comment.
On peut enlever ce if non ? le if is_field_completed(json_data,'$.initialAlert.reporting'): dans add_Case_priority va déjà le vérifier a priori
There was a problem hiding this comment.
Yes effectivement, on vérifie la condition par la suite de manière détournée, j'ai l'impression que ça représente un peu plus la logique de le garder, et d'éviter de passer dans les sous fonctions si ce n'est pas nécessaire
| try: | ||
| jsonpath_expr = parse(json_path) | ||
| return len(jsonpath_expr.find(json_data))>=1 | ||
| except Exception as e: | ||
| print(f"Error raised in is_field_completed : {e}") | ||
| raise | ||
|
|
||
| def get_field_value(json_data: Dict[str, Any], json_path: str): | ||
| try: | ||
| isCompleted = is_field_completed(json_data, json_path) | ||
|
|
||
| if not isCompleted: | ||
| return None | ||
|
|
||
| jsonpath_expr = parse(json_path) | ||
| matches = jsonpath_expr.find(json_data) | ||
|
|
||
| if len(matches) > 1: | ||
| return [match.value for match in matches] | ||
| return matches[0].value | ||
|
|
||
| except Exception as e: | ||
| print(f"Error raised in is_field_completed : {e}") | ||
| raise |
There was a problem hiding this comment.
Vraiment top tout ça !! Tu peux les intégrer vite fait dans la partie to_cisu aussi non ?
|
muffoltz
force-pushed
the
feat/cisu-converter
branch
from
7cce5dc to
0a5b59b
Compare
|
|
(rules regarding RS-MAJ are out of scope)