-
Notifications
You must be signed in to change notification settings - Fork 863
Description
The context
https://github.com/grpc-ecosystem/grpc-spring/blob/master/grpc-client-spring-boot-autoconfigure/src/main/java/net/devh/boot/grpc/client/config/GrpcChannelProperties.java#L75 , L89, and L101-L104 inclusive describe the dns
scheme for channel URLs, seemingly to reflect official gRPC docs from https://github.com/grpc/grpc/blob/master/doc/naming.md
The bug
Current docs define a template of schema:[//[authority]][/path]
and call the scheme dns:/
with and describe it as follows:
* <li>{@code dns:/localhost (might refer to the IPv4 or the IPv6 address or both, dependent on the system
* configuration, it does not check whether there is actually someone listening on that network interface)}</li>
* <li>{@code dns:/example.com}</li>
* <li>{@code dns:/example.com:9090}</li>
* <li>{@code dns:///example.com:9090}</li>
, which has the template wrong (it's not "schema" but "scheme", it's actually scheme:[//authority]/path
for dns
scheme and might be just scheme:location
or scheme://location
for other schemes), doesn't showcase the "DNS authority" version at all, and also contradicts the official referenced gRPC docs in the three first cases. However, since the actual code implementation of gRPC for Java actually shows that it's not a host
(as the official docs would suggest) but a path
instead, and require that it should start with /
, this makes the official docs invalid and grpc-spring docs slightly invalid by referencing those docs and not mentioning this problem. IMVHO this discrepancy should be mentioned explicitly. Also, the scheme should be named just dns
and not dns:/
, because the colon and slash are not part of the URL scheme name.
BTW, current doc also has minor other problems, like having the @code
too large in the first case (shouldn't wrap the parenthesized remark, it's not code), doesn't explain the ///
idiom at all (official docs do it for a good reason, it's really easy to misunderstand it), spells URI in lowercase etc.
Additional context
This doc was added in 900c651 and further in 48e91fc . The gRPC implementation checks this at https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java#L56
BTW, if "pull requests are welcome", I'll gladly fix this one and do a PR for it, since it's a trivial doc update TBH.