[Go] Rewrite of Go Client, Server generator #5037
Conversation
|
@guohuang @neilotoole @wy-z @vpolouchkine @ePaul @xavier-fernandez This replaces PR #4440, it takes care of a few other issues I ran into as the API I am using evolves. I was hoping to find a better solution for authentication but have been unable to come up with one. I re-implemented the Configuration also so it is more familiar with the other languages. Not sure why Shippable is failing, cannot see the error directly? |
The shippable error has been worked around by #5031 by commenting the tests with OpenJDK8. The issue has nothing to do with our changes and I'll report the issue to Shippable later if it's still not resolved. For the time being, please ignore the Shippable error. |
|
The |
| // create path and map variables | ||
| localVarPath := a.Configuration.BasePath + "{{path}}"{{#pathParams}} | ||
| localVarPath := a.client.cfg.BasePath + "{{path}}"{{#pathParams}} |
There was a problem hiding this comment.
For {{path}}, please use unescaped value, i.e. {{{path}}}
| @@ -97,7 +97,7 @@ public GoClientCodegen() { | |||
| typeMapping.put("boolean", "bool"); | |||
| typeMapping.put("string", "string"); | |||
| typeMapping.put("UUID", "string"); | |||
| typeMapping.put("date", "time.Time"); | |||
| typeMapping.put("date", "SwaggerDateType"); | |||
There was a problem hiding this comment.
@antihax Shall we keep time.time so that the auto-generated code at least compile?
There was a problem hiding this comment.
Did it fail to compile with the template? time.Time used in this manner will cause a runtime error.
There may be a better way to handle it?
There was a problem hiding this comment.
What about mapping it as "string" instead?
There was a problem hiding this comment.
That is a possibility.
I have an app in production using the date type so it should help determine any issues that arise from using string.
I will test that tonight and convert if needed.
There was a problem hiding this comment.
String worked with minimal changes for the user. It can be converted by using
dateObject, err := time.Parse("2006-01-02", dateString)
| {{#isApiKey}}- **Type**: API key | ||
| - **API key parameter name**: {{{keyParamName}}} | ||
| - **Location**: {{#isKeyInQuery}}URL query string{{/isKeyInQuery}}{{#isKeyInHeader}}HTTP header{{/isKeyInHeader}} | ||
| !!! NOT IMPLEMENTED !!! |
There was a problem hiding this comment.
@antihax so API key (via header or query parameter) is no longer supported?
There was a problem hiding this comment.
That is correct. I could not see in the original where it was supported as a reference of how to implement it.
I could take another look at some other implementations and try to determine. Is there a way to write tests for it?
There was a problem hiding this comment.
I recall how I became confused here...
API Key in Query String: https://github.com/antihax/swagger-codegen/blob/New-GoClient-GoServer/samples/client/petstore/go/go-petstore/pet_api.go#L99
I will add the header equivalent tonight and alter the documentation.
|
@antihax when you've time, I wonder if you can help resolve the merge conflicts and then I'll merge the enhancements into 2.3.0. Thanks! |
|
Seems like issues with CIs (shippable, Appveyor) at the moment. Will restart the jobs tomorrow to see if the issue goes away. |
|
Looks like an actual error that Travis-CI did not find. Trying to
understand it.
```
Exception in thread "main" java.lang.RuntimeException: Could not generate api file for 'Pet'
at io.swagger.codegen.DefaultGenerator.generateApis(DefaultGenerator.java:463)
at io.swagger.codegen.DefaultGenerator.generate(DefaultGenerator.java:666)
at io.swagger.codegen.cmd.Generate.run(Generate.java:234)
at io.swagger.codegen.SwaggerCodegen.main(SwaggerCodegen.java:41)
Caused by: com.samskivert.mustache.MustacheParseException: Section close tag with mismatched open tag 'pathParams' != 'operation' @ line 42
```
|
|
Ah interesting... I'll also take a look over the weekend. |
Correct missing line from resolving conflicts.
|
Saw your fix so shippable did its job in catching issues related to mustache (e.g. missing tags) |
wing328
changed the title
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)2.3.0branch for breaking (non-backward compatible) changes.Description of the PR
Rewrite of Go client. This will need community review. This is modeled after some other API clients (go-github, godo, etc).
This replaces #4440