Add Possibility to increase max receive/send message size #149
Conversation
There was a problem hiding this comment.
Thanks @davidelienhard! One question just to make sure I understand what this is doing, but the approach LGTM.
sdk.go
Outdated
| @@ -141,7 +149,7 @@ func Serve(fn v1beta1.FunctionRunnerServiceServer, o ...ServeOption) error { | |||
| return errors.Wrapf(err, "cannot listen for %s connections at address %q", so.Network, so.Address) | |||
| } | |||
|
|
|||
| srv := grpc.NewServer(grpc.MaxRecvMsgSize(so.MaxMsgSize), grpc.MaxSendMsgSize(so.MaxMsgSize), grpc.Creds(so.Credentials)) | |||
| srv := grpc.NewServer(grpc.MaxMsgSize(so.MaxMsgSize), grpc.MaxRecvMsgSize(so.MaxMsgSize), grpc.MaxSendMsgSize(so.MaxMsgSize), grpc.Creds(so.Credentials)) | |||
There was a problem hiding this comment.
What's the difference between max size vs max recv size and max send size?
Does Crossplane (the client) need to be configured to agree on message size with the function (the server)?
There was a problem hiding this comment.
Hi, maxSize is actually equal to max recv size, it is going to be deprecated (we could already remove it actually, I had done it but in the last commit apparently I forgot it and added it again by mistake)
The difference between receive and send size is:
- receive: if the incoming message exceeds the size it refuses to read the message
- send: it cannot send messages larger than the maximum
There was a problem hiding this comment.
I'm adding a commit deleting the grpc.MaxMsgSize(so.MaxMsgSize) since it is going to be deprecated
There was a problem hiding this comment.
Based on your comment I saw on the documentation that the default value for sendMsgSize is math.MaxInt32 and not the same as recvsize, therefore i modified the repo adding a commit, now the serveOption just modifies the RecvMsgSize and not the Send as well
|
Thanks! This looks good, but it's not passing DCO. Could you squash down to one commit, and make sure that commit is signed off. Once that's done I can merge. |
|
Hi, I had to close this pull request and am opening a new one with the modifications you said This is the link |
Description of your changes
Fixes #
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested