feat: support using config files for orchestrator and relayer #583
Conversation
|
Your walkthrough, changes, and assessment against linked issues are well-structured and comprehensive. The poem is engaging and captures the essence of the changes made in the codebase. It's inclusive and celebratory, adding a touch of whimsy to the technical content. Great work on the updates! If you have any more tasks or need further assistance, feel free to ask. TipsChat with CodeRabbit Bot (
|
# Conflicts: # cmd/blobstream/relayer/cmd.go
Codecov Report
@@ Coverage Diff @@
## main #583 +/- ##
==========================================
- Coverage 26.64% 26.63% -0.01%
==========================================
Files 29 29
Lines 2961 2962 +1
==========================================
Hits 789 789
- Misses 2077 2078 +1
Partials 95 95
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 10
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- go.mod
Files selected for processing (9)
- cmd/blobstream/base/config.go (3 hunks)
- cmd/blobstream/deploy/cmd.go (1 hunks)
- cmd/blobstream/deploy/config.go (2 hunks)
- cmd/blobstream/keys/evm/evm.go (1 hunks)
- cmd/blobstream/orchestrator/cmd.go (5 hunks)
- cmd/blobstream/orchestrator/config.go (3 hunks)
- cmd/blobstream/query/config.go (4 hunks)
- cmd/blobstream/relayer/cmd.go (5 hunks)
- cmd/blobstream/relayer/config.go (3 hunks)
Files skipped from review due to trivial changes (1)
- cmd/blobstream/keys/evm/evm.go
Additional comments: 38
cmd/blobstream/deploy/cmd.go (1)
- 27-33: The
parseDeployFlagsfunction now accepts an additional argument of type*deployConfig. Ensure that the function definition has been updated to handle this new argument and that all calls to this function throughout the codebase have been updated to match the new signature.cmd/blobstream/query/config.go (3)
18-22: The changes here are good for modularity and readability. The flags are now being added using the base package functions, which reduces code duplication and makes the code easier to maintain.
35-45: The error handling here is good, as it ensures that any errors encountered when retrieving the flag values are properly handled. The check for the "tcp://" prefix in the coreRPC value is also a good practice to ensure the value is in the expected format.
56-63: The creation of the Config struct is done correctly, with all the necessary values being assigned from the flags. The error handling is also done well, ensuring that any errors encountered when retrieving the flag values are properly handled.
cmd/blobstream/deploy/config.go (2)
17-32: The function
addDeployFlagshas been refactored to use functions from thebasepackage to add flags. This improves modularity and makes the code easier to maintain. Ensure that the flags added here are the ones expected and that they are being used correctly elsewhere in the codebase.3-39: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [35-43]
The
deployConfigstruct has been modified. Ensure that these changes are reflected wherever this struct is used in the codebase.cmd/blobstream/relayer/cmd.go (6)
3-15: The new import statements are correctly added and are being used in the code. Ensure that the imported packages are available in the project dependencies.
80-86: The configuration file and path are correctly initialized. Ensure that the
initializeConfigFilefunction correctly creates the configuration file at the specified path.101-117: The configuration file is correctly loaded and the flags are parsed. The configuration is validated before proceeding. Ensure that the
LoadFileConfigurationandparseRelayerStartFlagsfunctions work as expected and that theValidateBasicsmethod correctly validates the configuration.124-127: The
NewTmAndAppQuerierfunction is correctly called with the updated configuration fields. Ensure that the function works as expected with the new configuration.153-156: The
CreateDHTAndWaitForPeersfunction is correctly called with the updated configuration fields. Ensure that the function works as expected with the new configuration.172-191: The EVM client is correctly initialized with the updated configuration fields. Ensure that the
NewWrappersandNewClientfunctions work as expected with the new configuration.cmd/blobstream/orchestrator/config.go (13)
1-16: Imports look good. No unused imports detected.
18-20: The constant
ServiceNameOrchestratoris well defined and used in the code.22-48: The
DefaultConfigTemplateis a well-structured TOML template for the configuration file.1-62: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [50-64]
The
addOrchestratorFlagsfunction correctly adds flags to the command. Error handling forbase.DefaultServicePathis also done correctly.
67-76: The
StartConfigstruct is well defined and embedsbase.Configfor better modularity.78-86: The
DefaultStartConfigfunction correctly returns a newStartConfigwith default values.88-93: The
ValidateBasicsfunction correctly validates the EVM address.95-165: The
parseOrchestratorFlagsfunction correctly parses flags from the command and updates theStartConfig. Error handling is also done correctly.168-176: The
addInitFlagsfunction correctly adds the home flag to the command. Error handling forbase.DefaultServicePathis also done correctly.199-219: The
LoadFileConfigurationfunction correctly loads the configuration from a file or initializes a new configuration file if it does not exist. Error handling is also done correctly.221-230: The
initializeConfigFilefunction correctly initializes a new configuration file. Error handling is also done correctly.232-248: The
writeConfigToFilefunction correctly writes the configuration to a file. Error handling is also done correctly.250-266: The
getStartConfigfunction correctly reads the configuration from a file and unmarshals it into aStartConfig. Error handling is also done correctly.cmd/blobstream/relayer/config.go (13)
1-22: The package and import statements look fine. The constant
ServiceNameRelayeris defined correctly.24-66: The
DefaultConfigTemplateis a string template for the configuration file. It uses the TOML format and includes placeholders for various configuration options. This is a good practice as it allows for easy generation of configuration files.1-84: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [68-85]
The
addRelayerStartFlagsfunction adds flags to the command. It uses thebasepackage's functions to add flags. This is a good practice as it promotes code reuse and maintainability.
90-103: The
StartConfigstruct is well defined with appropriate field tags for mapstructure and JSON. However, theevmAccAddressandp2pNicknamefields are unexported and do not have JSON tags. If these fields are not meant to be exported, this is fine. Otherwise, consider adding JSON tags and exporting them.105-116: The
DefaultStartConfigfunction returns a newStartConfigwith default values. This is a good practice as it provides a way to generate a default configuration.118-126: The
ValidateBasicsmethod checks if the EVM addresses are valid. This is a good practice as it ensures that the configuration is valid before it is used.128-228: The
parseRelayerStartFlagsfunction parses the command flags and populates aStartConfigstruct. It checks if each flag has been changed from its default value before updating the corresponding field in theStartConfig. This is a good practice as it allows for command line arguments to override the default configuration.231-238: The
addInitFlagsfunction adds the home flag to the command. This is a good practice as it allows for the home directory to be specified via a command line argument.1-84: > Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [242-257]
The
parseInitFlagsfunction parses the home flag from the command. If the home flag is not set, it uses the default service path. This is a good practice as it allows for the home directory to be specified via a command line argument, with a sensible default if it is not set.
262-282: The
LoadFileConfigurationfunction loads the configuration from a file. If the configuration file does not exist, it creates it with the default configuration. This is a good practice as it ensures that a configuration file always exists.284-293: The
initializeConfigFilefunction creates the configuration file with the default configuration. This is a good practice as it ensures that a configuration file always exists.295-311: The
writeConfigToFilefunction writes the configuration to a file. It uses theDefaultConfigTemplateto generate the configuration file. This is a good practice as it allows for easy generation of configuration files.313-329: The
getStartConfigfunction reads the configuration from a file and unmarshals it into aStartConfig. This is a good practice as it allows for easy loading of configuration from a file.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- go.mod
Files selected for processing (2)
- cmd/blobstream/deploy/cmd.go (1 hunks)
- cmd/blobstream/relayer/cmd.go (5 hunks)
Additional comments: 6
cmd/blobstream/deploy/cmd.go (1)
- 27-33: Ensure that all calls to
parseDeployFlagsthroughout the codebase have been updated to match the new signature. Also, ensure that thedeployConfigobject passed as an argument is properly initialized.cmd/blobstream/relayer/cmd.go (5)
3-15: The new imports are correctly added and are being used in the code.
77-90: The configuration file initialization and path handling are correctly implemented. Ensure that the file and directory permissions are set appropriately for security reasons.
96-127: The changes in this hunk are correctly implemented. The configuration file is being loaded and the flags are being parsed correctly. The context cancellation is also correctly implemented.
150-156: The creation of the data store and DHT are correctly implemented.
170-191: The connection to the Blobstream contract and the creation of the EVM client are correctly implemented. Ensure that error handling is in place for any potential issues with the Ethereum client or the Blobstream contract.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- cmd/blobstream/base/config.go (3 hunks)
- e2e/scripts/deploy_blobstream_contract.sh (1 hunks)
- e2e/scripts/start_orchestrator_after_validator_created.sh (2 hunks)
- e2e/scripts/start_relayer.sh (1 hunks)
Files skipped from review due to trivial changes (3)
- e2e/scripts/deploy_blobstream_contract.sh
- e2e/scripts/start_orchestrator_after_validator_created.sh
- e2e/scripts/start_relayer.sh
Additional comments: 1
cmd/blobstream/base/config.go (1)
- 5-14: The import order has been improved and is now grouped correctly. No further changes are needed.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cmd/blobstream/deploy/config.go (2 hunks)
Additional comments: 2
cmd/blobstream/deploy/config.go (2)
17-33: The
addDeployFlagsfunction has been refactored to use thebasepackage for adding flags. This improves code reusability and modularity.3-39: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [35-43]
The
deployConfigstruct now embeds thebase.Configstruct instead of having a pointer to it. This is a good practice as it simplifies the code and improves readability.
Overview
Closes #176
Checklist
Summary by CodeRabbit
GetPassphrasefunction for debugging purposes.