Add grpc helpers to go#35
Merged
smacker merged 6 commits intosrc-d:masterfrom Nov 28, 2018
smacker:add_grpc_helpers_to_go
Merged
Add grpc helpers to go#35smacker merged 6 commits intosrc-d:masterfrom smacker:add_grpc_helpers_to_go
smacker merged 6 commits intosrc-d:masterfrom
smacker:add_grpc_helpers_to_go
Conversation
Contributor
Author
|
Required by src-d/lookout-gometalint-analyzer#17 |
carlosms
approved these changes
Nov 23, 2018
Contributor
carlosms
left a comment
There was a problem hiding this comment.
LGTM, with minor comments
language-analyzer.go
Outdated
| var dataSrvAddr = "localhost:10301" | ||
| var dataSrvAddr, _ = pb.ToGoGrpcAddress("ipv4://localhost:10301") | ||
| var version = "alpha" | ||
| var maxMessageSize = 100 * 1024 * 1024 //100mb |
Contributor
There was a problem hiding this comment.
nitpicking, but let's change this comment to 100MB
pb/grpc.go
Outdated
| ) | ||
|
|
||
| // maxMessageSize overrides default grpc max. message size to send/receive to/from clients | ||
| var maxMessageSize = 100 * 1024 * 1024 // 100mb |
Contributor
There was a problem hiding this comment.
same, let's change this comment to 100MB
Contributor
Author
|
let's keep it open until there is a PR with python helpers to make sure they aren't too different. |
Contributor
|
I'd merge and create an issue to implement it in a next iteration |
Contributor
Author
|
I'm currently working on python implementation. But I'm afraid it can be much different from Go one. It's better to review both of them before merging. To make sure they are consistent and decide which inconsistencies are acceptable (languages are different so the code can't be absolutely the same) |
Signed-off-by: Maxim Sukharev <max@smacker.ru>
Signed-off-by: Maxim Sukharev <max@smacker.ru>
Signed-off-by: Maxim Sukharev <max@smacker.ru>
Signed-off-by: Maxim Sukharev <max@smacker.ru>
Analyzer server doesn't need increased grpc message size It sends comments back, not UAST. Signed-off-by: Maxim Sukharev <max@smacker.ru>
Analyzers don't need it and there is no equivalent in python-sdk Signed-off-by: Maxim Sukharev <max@smacker.ru>
Contributor
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move low-level gRPC helpers from lookout to sdk.
Currently, URI schema is a huge mess. Some analyzers support RFC 3986 others not. Infra suffered from it already. This PR brings helpers to sdk and mention them as best-practice.
We allow to change
maxMessageSizein lookoutd but it doesn't make much sense to have increased size only on server or client side. Better if they always match.Similar helpers are implemented in python by #37.