Skip to content
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

[suggestion] Move logger.terminal_colors setting to the CLI layer #4030

Closed
Tracked by #392
0x009922 opened this issue Nov 3, 2023 · 1 comment
Closed
Tracked by #392

[suggestion] Move logger.terminal_colors setting to the CLI layer #4030

0x009922 opened this issue Nov 3, 2023 · 1 comment
Assignees
Labels
config-changes Changes in configuration and start up of the Iroha iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@0x009922
Copy link
Contributor

0x009922 commented Nov 3, 2023

Description

I propose to move logger.terminal_colors out of the configuration file to the layer of application's CLI args parsing, which happens before the config file is read and parsed.

Why

"Terminal colors" configure whether the overall Command Line Interface should be colorized or not. Therefore, it belongs to the layer of CLI args, not the config file. Moreover, I don't see a reason why logger output colors in terminal should differ from any other terminal output of Iroha.

Also, configuration parsing happens after CLI arguments parsing. The process of configuration parsing itself might involve logging of hints/warnings/errors with or without colors. If terminal_colors is set in the config file, it will be only available after the config is parsed. Thus, this option should be known before configuration parsing stage.

Changes

  • Remove logger.terminal_colors parameter from config file
  • Introduce --terminal-colors CLI argument with a corresponding ENV var (e.g. TERMINAL_COLORS)

Also

  • Currently terminal colors is automatically enabled/disabled accordingly to TTY color detection (if logger.terminal_colors is not set). This behaviour will stay the same.
@0x009922 0x009922 added iroha2-dev The re-implementation of a BFT hyperledger in RUST config-changes Changes in configuration and start up of the Iroha labels Nov 3, 2023
@6r1d
Copy link
Contributor

6r1d commented Nov 3, 2023

Looks good. I wonder if there's some weird way to connect to Docker or SSH that'll filter colours and be inconvenient.

0x009922 added a commit that referenced this issue Dec 12, 2023
…figuration (#4100)

Co-authored-by: ⭐️NINIKA⭐️ <DCNick3@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this issue Jan 9, 2024
…ha#4064, hyperledger-iroha#4079: re-architect logger and dynamic configuration (hyperledger-iroha#4100)

Co-authored-by: ⭐️NINIKA⭐️ <DCNick3@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this issue Jan 22, 2024
…ha#4064, hyperledger-iroha#4079: re-architect logger and dynamic configuration (hyperledger-iroha#4100)

Co-authored-by: ⭐️NINIKA⭐️ <DCNick3@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Signed-off-by: Asem-Abdelhady <asemshawkey@gmail.com>
@0x009922 0x009922 closed this as completed Feb 6, 2024
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this issue Feb 9, 2024
…ha#4064, hyperledger-iroha#4079: re-architect logger and dynamic configuration (hyperledger-iroha#4100)

Co-authored-by: ⭐️NINIKA⭐️ <DCNick3@users.noreply.github.com>
Signed-off-by: Dmitry Balashov <43530070+0x009922@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config-changes Changes in configuration and start up of the Iroha iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

2 participants