Add grpc helpers to go #35
Merged
Conversation
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
Contributor
Author
|
Required by src-d/lookout-gometalint-analyzer#17 |
carlosms
approved these changes
Nov 23, 2018
Contributor
carlosms
left a comment
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 <[email protected]>
Signed-off-by: Maxim Sukharev <[email protected]>
Signed-off-by: Maxim Sukharev <[email protected]>
Signed-off-by: Maxim Sukharev <[email protected]>
Analyzer server doesn't need increased grpc message size It sends comments back, not UAST. Signed-off-by: Maxim Sukharev <[email protected]>
Analyzers don't need it and there is no equivalent in python-sdk Signed-off-by: Maxim Sukharev <[email protected]>
Contributor
Author
|
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.