-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add log-level to launcher + fix trace debug #1757
Conversation
WalkthroughThe codebase has undergone a series of changes focused on logging functionality. The Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (6)
- go/enclave/components/block_processor.go (1 hunks)
- testnet/launcher/cmd/cli.go (3 hunks)
- testnet/launcher/cmd/cli_flags.go (2 hunks)
- testnet/launcher/cmd/main.go (1 hunks)
- testnet/launcher/config.go (2 hunks)
- testnet/launcher/docker.go (2 hunks)
Additional comments: 11
testnet/launcher/cmd/main.go (1)
- 23-23: The addition of the
logLevel
configuration option to the launcher is correctly implemented.testnet/launcher/cmd/cli_flags.go (2)
- 12-12: The new
logLevelFlag
constant is correctly defined.- 26-26: The description for the
logLevelFlag
in thegetFlagUsageMap
function is added and matches the intended functionality.testnet/launcher/config.go (2)
- 19-19: The
logLevel
field is correctly added to theConfig
struct.- 74-78: The
WithLogLevel
function is correctly implemented to set thelogLevel
field in theConfig
struct.testnet/launcher/cmd/cli.go (3)
- 16-16: The
logLevel
field is correctly added to theTestnetConfigCLI
struct.- 31-31: The parsing of the
logLevel
flag from the command line is correctly implemented.- 41-41: The assignment of the parsed
logLevel
to theTestnetConfigCLI
struct is correct.go/enclave/components/block_processor.go (1)
- 118-118: The removal of the "ingestionType" log parameter from the "Block inserted successfully" log message is noted. Ensure that this change is reflected in any documentation or monitoring tools that rely on this log parameter.
testnet/launcher/docker.go (2)
- 65-65: The
Start
method of theTestnet
type now correctly uses thelogLevel
from the configuration.- 105-105: The
logLevel
is also correctly used in the configuration of the validator node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but couple comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- testnet/launcher/cmd/cli.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- testnet/launcher/cmd/cli.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- go/common/types.go (1 hunks)
Additional comments: 1
go/common/types.go (1)
- 187-189: The addition of a nil check in the
String
method of theChainFork
type is a good defensive programming practice to prevent potential nil pointer dereferences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Why this change is needed
https://github.com/ten-protocol/ten-internal/issues/2799
As per title.
What changes were made as part of this PR
Please provide a high level list of the changes made
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks