Skip to content

Conversation

@fmorency
Copy link
Contributor

This pull request refactors the IsGrpcPort function in pkg/utils/grpc.go to improve functionality and introduces a new GetNodeStatus function for more detailed gRPC port checks. It also updates imports to include new dependencies.

Refactoring and new functionality:

  • pkg/utils/grpc.go: Replaced the original IsGrpcPort implementation with a call to the newly added GetNodeStatus function, which checks the gRPC port by querying the node's status. The IsGrpcPort function now logs debug information when the port is ready.
  • pkg/utils/grpc.go: Added the GetNodeStatus function, which establishes a gRPC connection, queries the node's status using nodev1beta1.ServiceClient, and returns the response. This provides more granular information about the gRPC port's readiness.

Dependency updates:

  • pkg/utils/grpc.go: Imported log/slog for logging and cosmossdk.io/api/cosmos/base/node/v1beta1 for node status functionality. Removed unused grpc/connectivity import.

@fmorency fmorency requested review from Copilot and jgryffindor June 13, 2025 12:38
@fmorency fmorency self-assigned this Jun 13, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the existing gRPC port checking by replacing the old IsGrpcPort implementation with a new GetNodeStatus function that interrogates the node’s status via a gRPC call and subsequently updates the IsGrpcPort function to log debug information upon successful connectivity.

  • Refactored IsGrpcPort to call GetNodeStatus for a more detailed connectivity check.
  • Introduced GetNodeStatus to establish a gRPC connection and fetch node status.
  • Updated imports by removing an unused dependency and adding new ones for logging and node status functionality.
Comments suppressed due to low confidence (1)

pkg/utils/grpc.go:13

  • Consider adding a documentation comment above GetNodeStatus to explain its purpose, parameters, and expected behavior for future maintainability.
func GetNodeStatus(target string) (*nodev1beta1.StatusResponse, error) {

Copy link

@jgryffindor jgryffindor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK!

@fmorency fmorency merged commit 22f7680 into manifest-network:main Jun 13, 2025
3 checks passed
@fmorency fmorency deleted the fix-grpc-test branch June 13, 2025 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants