feat: support getting config from homes for query commands #619
Conversation
WalkthroughThe changes across various Changes
Assessment against linked issues
Poem
TipsChat with CodeRabbit Bot (
|
![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: 5
Configuration used: CodeRabbit UI
Files selected for processing (5)
- cmd/blobstream/orchestrator/cmd.go (2 hunks)
- cmd/blobstream/orchestrator/config.go (2 hunks)
- cmd/blobstream/query/cmd.go (6 hunks)
- cmd/blobstream/query/config.go (2 hunks)
- cmd/blobstream/relayer/config.go (2 hunks)
Additional comments: 15
cmd/blobstream/orchestrator/config.go (3)
282-286: The function
GetStartConfighas been correctly renamed to follow Go's convention for exporting functions. Ensure that all references to this function in the codebase have been updated to use the new name.246-249: The update to use
GetStartConfiginLoadFileConfigurationis correct. However, ensure that the error message is consistent with the context of the function call and provides enough information for debugging.279-279: The file permissions for the configuration file are set to 0o600, which is a secure practice as it restricts access to the file owner only.
cmd/blobstream/query/cmd.go (6)
6-16: The new import statements are correctly added and used within the code.
501-574: The
tryToGetExistingConfigfunction is well-implemented with proper error handling and clear logic for retrieving configuration based on the service type.60-70: The integration of
tryToGetExistingConfigwithin theSignersfunction is correct, and the error handling for configuration retrieval is properly implemented.352-362: The integration of
tryToGetExistingConfigwithin theSignaturefunction is correct, and the error handling for configuration retrieval is properly implemented.88-96: The creation of Tendermint and application queriers within the
Signersfunction is correctly implemented, and the error handling is appropriate.380-383: The creation of Tendermint and application queriers within the
Signaturefunction is correctly implemented, and the error handling is appropriate.cmd/blobstream/query/config.go (4)
20-24: The addition of new flags for the command is correctly implemented and follows the established pattern for adding flags in Cobra-based CLI applications.
34-41: The
DefaultConfigfunction is correctly implemented and provides a clear and concise way to generate a default configuration.44-86: The
parseFlagsfunction is updated to modify thestartConfbased on the provided flags. However, the function signature in the summary mentions it should return a boolean indicating if any flag has changed, which is not reflected in the code. Ensure that the implementation aligns with the intended behavior as described in the summary.89-96: The
getP2PNodeFlagfunction is correctly implemented to retrieve the P2P node flag value and a boolean indicating if the flag has changed.cmd/blobstream/relayer/config.go (2)
345-349: The function
getStartConfighas been correctly renamed toGetStartConfigto make it publicly accessible. Ensure that all references to this function in other packages have been updated to use the new name.342-342: The use of
0o600permissions for the configuration file is a good security practice, ensuring that only the owner can read and write to the file.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cmd/blobstream/query/cmd.go (6 hunks)
Additional comments: 6
cmd/blobstream/query/cmd.go (6)
6-16: The new imports are necessary for the functionality introduced in the file, such as file path operations and configuration management.
58-74: The Signers function has been updated to use the new configuration retrieval logic, which is a good improvement for maintainability and usability.
350-366: The Signature function has been updated to use the new configuration retrieval logic, which is a good improvement for maintainability and usability.
526-526: There is an inconsistency in the field name
grpcInsecurebetween the orchestrator and relayer configurations. Ensure that this is intentional and does not lead to bugs due to mismatched field names.538-538: There is an inconsistency in the field name
grpcInsecurebetween the orchestrator and relayer configurations. Ensure that this is intentional and does not lead to bugs due to mismatched field names.501-578: The addition of the
tryToGetExistingConfigfunction is a good practice to simplify the configuration process for users by attempting to retrieve existing configurations from home directories based on the service type.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #619 +/- ##
=======================================
Coverage 26.13% 26.13%
=======================================
Files 29 29
Lines 3019 3019
=======================================
Hits 789 789
Misses 2135 2135
Partials 95 95 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- cmd/blobstream/query/cmd.go (6 hunks)
- cmd/blobstream/query/config.go (2 hunks)
Additional comments: 10
cmd/blobstream/query/cmd.go (6)
6-16: The addition of new import statements aligns with the changes described in the summary, ensuring the necessary packages are available for the new functionality.
61-70: The creation of a logger and the invocation of
tryToGetExistingConfigwithin theSignersfunction are implemented correctly, with proper error handling.88-96: The invocation of
common.NewTmAndAppQuerierin theSignersfunction correctly includes the additional parameterconfig.grpcInsecure, as per the changes described in the summary.353-362: The creation of a logger and the invocation of
tryToGetExistingConfigwithin theSignaturefunction are implemented correctly, with proper error handling.380-383: The invocation of
common.NewTmAndAppQuerierin theSignaturefunction correctly includes the additional parameterconfig.grpcInsecure, as per the changes described in the summary.501-577: The
tryToGetExistingConfigfunction is well-structured and includes comprehensive error handling for different scenarios where the configuration might be retrieved from. It also provides a default configuration if none is found.cmd/blobstream/query/config.go (4)
53-95: The summary indicates that
parseFlagsshould return aConfigand a boolean indicating if any flag has changed, but the code still returns aConfigand anerror. Please verify if the summary or the code needs to be updated to reflect the intended behavior.34-50: The implementation of
NewPartialConfigandDefaultConfigfunctions aligns with the summary description and appears to be correct.98-104: The
getP2PNodeFlagfunction correctly retrieves the P2PNode flag value and indicates if it has been changed, as described in the summary.20-24: The
addFlagsfunction has been updated to include new flags, which is consistent with the changes outlined in the summary.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cmd/blobstream/relayer/config.go (2 hunks)
Additional comments: 2
cmd/blobstream/relayer/config.go (2)
360-364: The function
GetStartConfighas been correctly updated to be public, aligning with the summary of changes.357-357: The file permissions for the configuration file are set to
0o600, which is a secure practice for sensitive configuration files.
Overview
Closes #618
Checklist
Summary by CodeRabbit
New Features
Style