grpc-ecosystem / grpc-ecosystem/grpc-spring
Docs in GrpcChannelProperties not clear about `dns` scheme and have minor other problems
- Dominant language
- Java
- Stars
- 3.7k
- Forks
- 858
- PR merge metrics
- No merged PRs in 30d
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:
*
* configuration, it does not check whether there is actually someone listening on that network interface)}
*
*
*
, 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 https://github.com/grpc-ecosystem/grpc-spring/commit/900c651c693aeb235f8722261abc1484f274bfc0 and further in https://github.com/grpc-ecosystem/grpc-spring/commit/48e91fc82387cbf686ed09f66da1e2fb64aebc65 . 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.
Contributor guide
Assessment
This issue has not been assessed yet.