fix: Need to be able to specify different host for gRPC in TCK#877
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the host configuration in TckAgentCardProducer by introducing separate constants for the general host and the gRPC host, enabling independent configuration via environment variables. A review comment suggests that GRPC_HOST should fall back to the value of HOST instead of the hardcoded DEFAULT_HOST to ensure that the gRPC endpoint correctly follows a custom host setting when a specific gRPC host is not provided.
| private static final String host = getEnvOrDefault("SUT_HOST", "localhost:9999"); | ||
| private static final String DEFAULT_HOST = "localhost:9999"; | ||
| private static final String HOST = getEnvOrDefault("SUT_HOST", DEFAULT_HOST); | ||
| private static final String GRPC_HOST = getEnvOrDefault("SUT_GRPC_HOST", DEFAULT_HOST); |
There was a problem hiding this comment.
The GRPC_HOST should fall back to the value of HOST if SUT_GRPC_HOST is not specified. Currently, it defaults to DEFAULT_HOST (localhost:9999). This means that if a user configures SUT_HOST to a custom value but leaves SUT_GRPC_HOST unset, the gRPC endpoint will incorrectly point to localhost:9999 instead of the custom host specified in SUT_HOST.
| private static final String GRPC_HOST = getEnvOrDefault("SUT_GRPC_HOST", DEFAULT_HOST); | |
| private static final String GRPC_HOST = getEnvOrDefault("SUT_GRPC_HOST", HOST); |
There was a problem hiding this comment.
@gemini-code-assist updated, does it look better now?
There was a problem hiding this comment.
Yes, that looks correct. By using HOST as the default for GRPC_HOST, you ensure that if SUT_GRPC_HOST is not explicitly set, it will inherit the value from SUT_HOST (or the default localhost:9999 if neither is set), which is the expected behavior.
No description provided.