-
Notifications
You must be signed in to change notification settings - Fork 277
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
feat: improve config debug-ability #4456
Conversation
@DCNick3, I would appreciate if you can take a look at the procmacro I wrote here, in config/base/derive. |
6b96f4d
to
5af02bf
Compare
A bit concerned with the inclusion of source code location in the error trace (the |
I am concerned too. There are also complete backtraces attached, which are displayed in dev environment. Anyway, we can disable |
9b613a6
to
1aa3bf6
Compare
address hyperledger-iroha#4456 (comment) Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Co-authored-by: ⭐️NINIKA⭐️ <[email protected]> Signed-off-by: 0x009922 <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
rust-lang/rust#44752 (comment) Signed-off-by: Dmitry Balashov <[email protected]>
address hyperledger-iroha#4456 (comment) Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Dmitry Balashov <[email protected]>
ca08d9d
to
d3dbf2c
Compare
Description
This PR contains a major improvement of the work done in #4239. This is a big step towards better debug-ability of Iroha configuration system due to better errors. And yet, there are still many things to improve further!
Key points:
ConfigReader
API, which handles configuration sources in a dynamic manner rather than relying onserde
completely, thus giving high flexibility in how to handle different cases. It is also built targeting Iroha domain specifically and doesn't have any extra stuff. Much less boilerplate.error_stack
crate, which gives higher control over errors reporting thaneyre
does.--trace-config
works! Useslog
&stderrlog
crates for it. Seems to be not a heavy solution for this. It is completely separate fromtracing
stack.Further enhancements
There are still many things that might be improved.
error_stack
:format!
ConfigReader
, use better naming and abstractions, perhaps.ConfigReader
. There is lots of naive code and extra allocations.error_stack
. Sometimes it would be really cool to just "forward" underlying context without changing it, but currently it's howerror_stack
is designed. So, we have sometimes stacks like "Failed to load config -> Failed to load config -> failed to read file system -> ..."slim
compile feature for Iroha, which will remove--trace-config
support alongside withlog
&stderrlog
dependencies, and morePRIVATE_KEY_ALGORITHM
+PRIVATE_KEY_PAYLOAD
, and removing cratchyenv_custom
API fromConfigReader
Linked isues
Closes #3470, #4299, #4300
Gallery
Note: I've changed some bits after making these screenshots.
Some terminal screenshots
TODO
iroha_config_base
iroha_client_cli
compilation failurefails_with_no_trusted...
incli/src/lib.rs