Allow multiple thrift services for tchannel client #103
Conversation
|
It might be nice to change our client generation semantics to only generate the methods we need, with shorter names. If the client-config.json had a mapping of method names to generate, e.g. This will reduce the amount of code generated and will allow us to give shroter method names instead of the long form. |
| // {{.Name}} ... | ||
| func (c *{{$clientName}}) {{.Name}}( | ||
| // {{title $svc.Name}}{{title .Name}} is a client RPC call for method "{{.Name}}" of thrift service "{{$svc.Name}}" | ||
| func (c *{{$clientName}}) {{title $svc.Name}}{{title .Name}}( |
There was a problem hiding this comment.
This method name will get quite long for our generated clients.
|
@Raynos Agreed, I have thought about it, but ended up with the simplest solution here. I will try to make it happen. |
codegen/module_system.go
Outdated
| clientMethodName = method.DownstreamMethod.Name | ||
| if e.WorkflowType == "tchannelClient" { | ||
| clientMethodName = method.DownstreamService + strings.Title(clientMethodName) | ||
|
|
There was a problem hiding this comment.
Interesting, not sure how it gets in here
codegen/module_system.go
Outdated
| @@ -690,6 +691,16 @@ func (g *EndpointGenerator) generateEndpointFile( | |||
| strings.Title(method.Name) + "Endpoint" | |||
| } | |||
|
|
|||
| // TODO: http client needs to support multiple thrift services | |||
| var clientMethodName string | |||
| if method.DownstreamMethod != nil { | |||
There was a problem hiding this comment.
Can method.DownstreamMethod be nil here? (i don't think it can)
If it can, an error should be thrown here. If it can't, this if should be removed
There was a problem hiding this comment.
For a non-proxy custom endpoint, it can, and it's not an error.
There was a problem hiding this comment.
If it is nil, clientMethodName will be "" after this if. Shouldn't it be an error to render template with "" ClientMethodName?
There was a problem hiding this comment.
The usage of clientMethodName is guarded by a downstream existence check, it's only used when downstream exists.
|
lgtm. |
1 similar comment
No description provided.