Json output
pour généraliser les sorties json sur toutes les commandes
Merge request reports
Activity
requested review from @Nicolas80
assigned to @pokapow
mentioned in issue #47
added 12 commits
- 8760c3bd...3738e8b7 - 2 earlier commits
- 44073020 - * Adding `-c` crypto scheme parameter (with default ed25519) in different places
- da7c790d - * Added some logic between arguments of `vault import`; can't provide both...
- 8bf81cbc - * Adapted catch_known of predefined derivations to properly handle both...
- 0da71079 - * reverted most of the commit e9cd6a99: "Add secret format in database and display"
- bbd95707 - Added extra message when the DB parsing of DbAccountId fails, so we know for...
- f9beaf54 - * Updated version to 0.4.1
- 2843da7b - use json output for all usefull commands
- c4f8f3dd - Fixes after rebase of modified upstream branch
- f72566bd - * changes from `cargo fmt`
- a6bc5180 - * small fix for `vault use` with Json output format.
Toggle commit listadded 7 commits
-
503d4caa...4010a9e4 - 3 commits from branch
master
- 756be638 - use json output for all usefull commands
- 0a74e9a6 - changes from `cargo fmt`
- 6aa88af6 - * small fix for `vault use` with Json output format.
- a94ef7ec - Adapting `vault inspect`
Toggle commit list-
503d4caa...4010a9e4 - 3 commits from branch
added 1 commit
- 7b04c93a - Added simple tests of display::compute_vault_accounts_json method
added 1 commit
- f4e7026a - Added simple tests of display::compute_vault_accounts_json method
added 1 commit
- 4e7c3ed7 - Added new version 0.4.3-RC1 changelogs for Release Candidate.
added 1 commit
- cdc129ad - Added new version 0.4.3-RC1 changelogs for Release Candidate.
@HugoTrentesaux est-ce que tu veux faire un review de celle-ci avant merge ?
J'ai fais une release candidate postée sur le forum et je comptais attendre quelques jours avant de merge.
TL;DR : je désapprouve mais ne m'oppose pas
Je suis vraiment pas fan de la manière dont c'est implémenté. Plutôt que d'avoir un match(output_format) dans chaque command, j'aurais changé la valeur de retour pour un Result typé. Le type retourné doit implémenter les traits serialize_json (implémenté automatiquement) et serialize_human (implémenté manuellement soit avec un format!, soit avec des println!, mais proprement).
Ça implique de faire un type pour chaque valeur de retour, mais ça simplifie pas mal le développement.
Mais comme je n'ai pas eu le temps de faire ce retour pendant les deux mois où la MR était ouverte, je ne vais pas exprimer un avis bloquant.
Pour le serialize_json dont tu parle, j'imagine qu'il faudrait également prévoir un objet spécifique (à sérialiser en json) en cas d'erreur, un peu comme pour les API Rest avec le media type "application/problem+json".
Exemple venant d'un site fédéral belge qui donne notament des guidelines pour les API Rest:
https://www.belgif.be/specification/rest/api-guide/#error-handling
Bon, je ne sais pas trop si on veux prendre la même chose ici (ce n'est pas à proprement parler des API "Rest"
)Sinon, on fait quoi du coup pour cette MR; est-ce que @pokapow ou moi tentons de faire le changement ou ... ?
Edited by Nicolas80@HugoTrentesaux, @pokapow Est-ce que vous pourriez review le dernier commit 31cba144 ?
J'ai appliqué les changements suggérés par Hugo pour la commande
vault inspect
.A voir si cela vous semble correct avant de regarder pour adapter pour les autres commandes qui supportent le OutputFormat
json
.Exemple avec ce changement quand une GcliError est retournée:
format human (pas de changement)
gcli -o human vault inspect -v predef-sr//Aliceaze --no-password Input("No account found with name:'predef-sr' and path:'//Aliceaze'")
format json => converti en JsonOutputError et sérialisé en json
gcli -o json vault inspect -v predef-sr//Aliceaze --no-password | jq { "error_type": "Input", "error": "No account found with name:'predef-sr' and path:'//Aliceaze'" }
Qu'est-ce qui t'empêche de gérer l'affichage au plus haut niveau :
let result = match data.args.subcommand.clone() { /* ... */ }; print_command_output(data.args.output_format, result);
Avec le match sur le output format à l'intérieur en cas d'erreur également ?
À part l'
async
, peut-être ?Comme ça, tout ce qu'on attend des commandes c'est de retourner quelque chose qui se sérialise. Pas trop le temps de regarder ça plus en détail, mais la MR me semble aller dans la bonne direction en terme de lisibilité et maintenabilité.
added 1 commit
- 31cba144 - * Added SerializeJson & SerializeHuman traits
added 1 commit
- c7e0404b - * Adapted several commands to the new structure with SerializeHuman