use Go templates in protofile comments #1056

Jeremytjuh · 2019-10-02T11:17:08Z

Introduce GO Templates into proto comments to allow more advanced documentation such as from
external (markdown) files and automatically generated content based on the proto definitions.

For example:
The comments in this proto file:

syntax = "proto3";

package LoginTest;

import "google/api/annotations.proto";

service LoginService {

// Logout
// 
// {{.MethodDescriptorProto.Name}} is a call with the method(s) {{$first := true}}{{range .Bindings}}{{if $first}}{{$first = false}}{{else}}, {{end}}{{.HTTPMethod}}{{end}} within the "{{.Service.Name}}" service.
// It takes in "{{.RequestType.Name}}" and returns a "{{.ResponseType.Name}}".
// 
// ## {{.RequestType.Name}}
// | Field ID    | Name      | Type                                                       | Description                  |
// | ----------- | --------- | ---------------------------------------------------------  | ---------------------------- | {{range .RequestType.Fields}}
// | {{.Number}} | {{.Name}} | {{if eq .Label.String "LABEL_REPEATED"}}[]{{end}}{{.Type}} | {{fieldcomments .Message .}} | {{end}}  
//  
// ## {{.ResponseType.Name}}
// | Field ID    | Name      | Type                                                       | Description                  |
// | ----------- | --------- | ---------------------------------------------------------- | ---------------------------- | {{range .ResponseType.Fields}}
// | {{.Number}} | {{.Name}} | {{if eq .Label.String "LABEL_REPEATED"}}[]{{end}}{{.Type}} | {{fieldcomments .Message .}} | {{end}}  
rpc Logout (LogoutRequest) returns (LogoutReply) {
  option (google.api.http) = {
    post: "/v1/example/ekko"
    body: "*"
    additional_bindings {
      post: "/test/test/test"
    }
    };
  }
}

message LogoutRequest {
  // This field only contains this title
  string timeoflogout = 1;
  // This is the title
  //
  // This is the "Description" of field test
  // you can use as many newlines as you want
  //
  //
  // it will still format the same in the table
  int32 test = 2;
  // Array title
  repeated string stringarray = 3;
}

message LogoutReply {
  // Message that displays whether the logout was succesful or not
  string message = 1;
}

would generate the following documentation in Swagger UI:

or when imported in Postman:

codecov-io · 2019-10-02T11:19:59Z

Codecov Report

Merging #1056 into master will increase coverage by <.01%.
The diff coverage is 57.44%.

@@            Coverage Diff             @@
##           master    #1056      +/-   ##
==========================================
+ Coverage   53.84%   53.85%   +<.01%     
==========================================
  Files          41       41              
  Lines        4147     4176      +29     
==========================================
+ Hits         2233     2249      +16     
- Misses       1670     1681      +11     
- Partials      244      246       +2

Impacted Files	Coverage Δ
protoc-gen-grpc-gateway/descriptor/registry.go	`57.75% <0%> (-1.02%)`	⬇️
protoc-gen-swagger/main.go	`26.85% <0%> (-0.26%)`	⬇️
protoc-gen-swagger/genswagger/template.go	`57.18% <64.28%> (+0.21%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e652ba0...933dfcc. Read the comment docs.

johanbrandhorst · 2019-10-02T11:25:32Z

Hi Jeremy, thanks for this PR. A few thoughts:

The goal here is to have proto comments that are automatically kept in-sync with the code definitions, I assume?
Would it be possible to consider adding support for the other swagger documentation fields too?
See https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/proto/examplepb/a_bit_of_everything.proto#L187 for an example.

Jeremytjuh · 2019-10-02T14:15:06Z

Thanks for the fast reply Johan!

The main purpose is that you can automatically generate documentation from the proto definitions and that it allows importing larger portions of documentation from external files.
Good idea! Just added support for the other swagger documentation fields

johanbrandhorst · 2019-10-02T14:23:28Z

The main purpose is that you can automatically generate documentation from the proto definitions and that it allows importing larger portions of documentation from external files.
How does the second part of this work? I'm not sure what you mean.

Could we also add an example of this to one of our example files? We usually put an example of how to use a new feature into one of the proto files in the examples folder. Since this one has to be explicitly turned on (thanks for that by the way!), you'll need to add a new file and ensure it is generated with the appropriate flags. Let me know if you need any help with this.

Lastly, how do you feel about adding a small documentation page for this new feature? I think that would greatly help with adoption. Take a look in https://github.com/grpc-ecosystem/grpc-gateway/tree/master/docs/_docs for other pages and examples of what they can look like.

achew22 · 2019-10-02T14:59:58Z

Can you help me understand why it is preferable to have an unstructured description of the request response object in the swagger definition? Wouldn't it be better to improve the logic that pulls this information to fill in the request and response objects with more definition?

Jeremytjuh · 2019-10-03T07:11:57Z

Can you help me understand why it is preferable to have an unstructured description of the request response object in the swagger definition? Wouldn't it be better to improve the logic that pulls this information to fill in the request and response objects with more definition?

Hi Andrew,
I want to introduce Go templates to protofile comments not just to be able to read the Request and Response objects, but also to make it possible to list information about an object you otherwise would not be able to list. With this feature you would also be able to use just one (for example) table "template" for multiple objects.

Jeremytjuh · 2019-10-03T08:00:35Z

The main purpose is that you can automatically generate documentation from the proto definitions and that it allows importing larger portions of documentation from external files.
How does the second part of this work? I'm not sure what you mean.

I have added a funcmap to the Go template which contains a function called "import". This function makes it possible to import documentation from an external file. For example a markdown file which contains a long detailed documentation. This way your protofile doesn't have to be flooded with documentation.

Could we also add an example of this to one of our example files? We usually put an example of how to use a new feature into one of the proto files in the examples folder. Since this one has to be explicitly turned on (thanks for that by the way!), you'll need to add a new file and ensure it is generated with the appropriate flags. Let me know if you need any help with this.

I'm trying to make an example file right now, but I have no experience with Bazel. So I would really appreciate if you could help me with that.

Lastly, how do you feel about adding a small documentation page for this new feature? I think that would greatly help with adoption.

Will do!

johanbrandhorst · 2019-10-03T09:15:04Z

I'm trying to make an example file right now, but I have no experience with Bazel. So I would really appreciate if you could help me with that.

Unfortunately, I, too, don't really undestand Bazel so the best I can say is "just do what the other example files do". @drigz or @achew22 might be able to help more.

drigz · 2019-10-03T15:59:30Z

I'm trying to make an example file right now, but I have no experience with Bazel. So I would really appreciate if you could help me with that.

Unfortunately, I, too, don't really undestand Bazel so the best I can say is "just do what the other example files do". @drigz or @achew22 might be able to help more.

My first question is whether it makes sense to add a new flag for this. Will anyone want to disable templating? On the one hand, maybe it would break people who have {{ }} in their comments already, but on the other hand that seems like it would not be common, and adding flags make maintenance harder. I'll leave that decision up to the two of you, though.

If you continue with the flag, you can follow the example of #944. You can test by installing Bazel, then running: (assuming you call the new target examplepb:examplepb_protoc_gen_swagger_template)

bazel build //examples/proto/examplepb:examplepb_protoc_gen_swagger_template

and inspecting bazel-bin/examples/proto/examplepb/a_bit_of_everything.swagger.json.

protoc-gen-swagger/main.go

johanbrandhorst

Waiting for the documentation page before merging.

FYI, don't be too worried about the generate job failing in CI, #1058 is currently blocking any other merges to master. As soon as that's in, just rebase on master and everything should be happy again.

googlebot · 2019-10-04T10:18:52Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

johanbrandhorst · 2019-10-04T15:06:56Z

Hi again @Jeremytjuh, please rebase on master to fix CI.

Jeremytjuh · 2019-10-07T07:20:49Z

Hi @johanbrandhorst,
I just rebased on master and the tests still fail. I'm not sure what I should do now.

johanbrandhorst · 2019-10-08T22:54:20Z

You'll also need to pull the latest generator image before regenerating the files again. See CONTRIBUTING.md. Sorry for the inconvenience 😅.

johanbrandhorst

Some small points. I think the bazel error is real:

ERROR: file 'examples/proto/examplepb/stream.swagger.json' is generated by these conflicting actions:
Label: //examples/proto/examplepb:examplepb_protoc_gen_swagger, //examples/proto/examplepb:examplepb_protoc_gen_swagger_template

The node test looks flaky, I will rerun it.

johanbrandhorst · 2019-10-09T08:21:20Z

examples/proto/examplepb/use_go_template.proto

+    post: "/v1/example/login"
+    body: "*"
+    };
+  }


Nit: the indentation here looks a bit off.

johanbrandhorst · 2019-10-09T08:22:15Z

docs/_docs/usegotemplates.md

+  *.proto 
+```
+
+For an example of a protofile which has Go templates enabled, [click here](https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/proto/examplepb/use_go_template.proto "Example protofile with Go template").


This is a great start! Do you think you could just add a protofile snippet and some screen shots as well to show what this looks like when rendered to swagger?

johanbrandhorst · 2019-10-09T08:51:29Z

Ah, the node_test error is because our runner doesn't have bzip2 installed. Might be a change in the underlying docker image we use, but should be easily fixable. I will fix it on master.

johanbrandhorst · 2019-10-09T09:17:44Z

node_test failure is gone.

drigz · 2019-10-09T14:47:22Z

ERROR: file 'examples/proto/examplepb/stream.swagger.json' is generated by these conflicting actions:
Label: //examples/proto/examplepb:examplepb_protoc_gen_swagger, //examples/proto/examplepb:examplepb_protoc_gen_swagger_template

I think this is due to a limitation/bug of protoc_gen_swagger(): it doesn't prefix its output files with the rule name, so it can't be used twice in the same directory (the single_file flag is an exception as it changes the shape of the output). My first suggestion would be to put the example in an adjacent subdirectory (eg examples/proto/templatepb) to avoid blocking this PR on finding a better solution.

If that's not easy, then we could just remove the Bazel rule for the example.

Co-Authored-By: Johan Brandhorst <[email protected]>

Jeremytjuh · 2019-10-11T12:26:09Z

Hi @johanbrandhorst,

All tests seem to be working now. Let me know if you want me to change anything.
Also, could you please let @googlebot know you consent as an author.
Thanks in advance!

johanbrandhorst · 2019-10-11T12:42:23Z

I consent to have my contributions to this PR merged into the repo under the relevant license. @achew22 could you make the bot happy please.

johanbrandhorst

LGTM, @achew22 if you have any more questions around this I'll leave it for a few days before merging.

Jeremytjuh · 2019-10-21T06:49:11Z

@johanbrandhorst @achew22 Are we good to go?

johanbrandhorst · 2019-10-21T08:45:39Z

@johanbrandhorst @achew22 Are we good to go?

I'm afraid we still need the CLA situation manually resolved by someone with authority. I've sent some reminders around. Sorry for the delay.

johanbrandhorst · 2019-10-21T09:09:57Z

@googlebot I consent

johanbrandhorst · 2019-10-21T12:33:39Z

@googlebot I consent.

ivucica · 2019-10-21T13:05:08Z

I am confused by the internal debugging state of the tool, but as I think I have the power to make this go through and as the consent has been granted, let's try.

googlebot · 2019-10-21T13:05:31Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

googlebot · 2019-10-21T13:07:44Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2019-10-21T13:11:13Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

johanbrandhorst · 2019-10-21T13:12:34Z

Alright we got there in the end. Thanks for your contribution @Jeremytjuh, sorry for the arcane bot treatment. I should know better than to make actual github suggestions on PRs.

ivucica · 2019-10-21T13:15:35Z

I didn't open a new internal bug for the tool, but I did fire off an email. Someone should do something!

* use GO templates in protofile comments * Added support for the other swagger documentation fields * Added feature for Go templates in imported files * Fixed typo in main.go Co-Authored-By: Johan Brandhorst <[email protected]> * Added Go template example * Added Go template documentation * Generated examples using build image * Generated bazel files using build image * Added use_go_templates.pb.gw.go in build file * Enhanced documentation * Fixed indents * Edited Go template bazel test * Excluded use_go_template.pb.gw.go in buildfile

googlebot added the cla: yes label Oct 2, 2019

johanbrandhorst changed the title ~~use GO templates in protofile comments~~ use Go templates in protofile comments Oct 2, 2019

johanbrandhorst reviewed Oct 4, 2019

View reviewed changes

protoc-gen-swagger/main.go Outdated Show resolved Hide resolved

johanbrandhorst requested changes Oct 4, 2019

View reviewed changes

googlebot added cla: no and removed cla: yes labels Oct 4, 2019

johanbrandhorst requested changes Oct 9, 2019

View reviewed changes

Jeremytjuh and others added 7 commits October 11, 2019 13:55

use GO templates in protofile comments

cfea81b

Added support for the other swagger documentation fields

ea7c4fa

Added feature for Go templates in imported files

b79f8ee

Fixed typo in main.go

05ef0a6

Co-Authored-By: Johan Brandhorst <[email protected]>

Added Go template example

7d5dc76

Added Go template documentation

890ca48

Generated examples using build image

f7496e5

Jeremytjuh added 6 commits October 11, 2019 13:55

Generated bazel files using build image

a31fecd

Added use_go_templates.pb.gw.go in build file

1485806

Enhanced documentation

1583586

Fixed indents

c894775

Edited Go template bazel test

d83b68e

Excluded use_go_template.pb.gw.go in buildfile

933dfcc

johanbrandhorst approved these changes Oct 11, 2019

View reviewed changes

ivucica added cla: yes and removed cla: no labels Oct 21, 2019

googlebot added cla: no and removed cla: yes labels Oct 21, 2019

ivucica added cla: yes and removed cla: no labels Oct 21, 2019

johanbrandhorst merged commit c3787b4 into grpc-ecosystem:master Oct 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use Go templates in protofile comments #1056

use Go templates in protofile comments #1056

Jeremytjuh commented Oct 2, 2019

codecov-io commented Oct 2, 2019 •

edited

Loading

johanbrandhorst commented Oct 2, 2019

Jeremytjuh commented Oct 2, 2019

johanbrandhorst commented Oct 2, 2019

achew22 commented Oct 2, 2019

Jeremytjuh commented Oct 3, 2019 •

edited

Loading

Jeremytjuh commented Oct 3, 2019

johanbrandhorst commented Oct 3, 2019

drigz commented Oct 3, 2019

johanbrandhorst left a comment

googlebot commented Oct 4, 2019

johanbrandhorst commented Oct 4, 2019

Jeremytjuh commented Oct 7, 2019

johanbrandhorst commented Oct 8, 2019

johanbrandhorst left a comment

johanbrandhorst Oct 9, 2019

johanbrandhorst Oct 9, 2019

johanbrandhorst commented Oct 9, 2019

johanbrandhorst commented Oct 9, 2019

drigz commented Oct 9, 2019 •

edited

Loading

Jeremytjuh commented Oct 11, 2019

johanbrandhorst commented Oct 11, 2019

johanbrandhorst left a comment

Jeremytjuh commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

ivucica commented Oct 21, 2019

googlebot commented Oct 21, 2019

googlebot commented Oct 21, 2019

googlebot commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

ivucica commented Oct 21, 2019

use Go templates in protofile comments #1056

use Go templates in protofile comments #1056

Conversation

Jeremytjuh commented Oct 2, 2019

codecov-io commented Oct 2, 2019 • edited Loading

Codecov Report

johanbrandhorst commented Oct 2, 2019

Jeremytjuh commented Oct 2, 2019

johanbrandhorst commented Oct 2, 2019

achew22 commented Oct 2, 2019

Jeremytjuh commented Oct 3, 2019 • edited Loading

Jeremytjuh commented Oct 3, 2019

johanbrandhorst commented Oct 3, 2019

drigz commented Oct 3, 2019

johanbrandhorst left a comment

Choose a reason for hiding this comment

googlebot commented Oct 4, 2019

johanbrandhorst commented Oct 4, 2019

Jeremytjuh commented Oct 7, 2019

johanbrandhorst commented Oct 8, 2019

johanbrandhorst left a comment

Choose a reason for hiding this comment

johanbrandhorst Oct 9, 2019

Choose a reason for hiding this comment

johanbrandhorst Oct 9, 2019

Choose a reason for hiding this comment

johanbrandhorst commented Oct 9, 2019

johanbrandhorst commented Oct 9, 2019

drigz commented Oct 9, 2019 • edited Loading

Jeremytjuh commented Oct 11, 2019

johanbrandhorst commented Oct 11, 2019

johanbrandhorst left a comment

Choose a reason for hiding this comment

Jeremytjuh commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

ivucica commented Oct 21, 2019

googlebot commented Oct 21, 2019

googlebot commented Oct 21, 2019

googlebot commented Oct 21, 2019

johanbrandhorst commented Oct 21, 2019

ivucica commented Oct 21, 2019

codecov-io commented Oct 2, 2019 •

edited

Loading

Jeremytjuh commented Oct 3, 2019 •

edited

Loading

drigz commented Oct 9, 2019 •

edited

Loading