feat(bazel): Add rule for generating .swagger.json files #613

mrmeku · 2018-04-24T05:58:51Z

Wraps protoc and protoc-gen-swagger in a bazel rule so that other rules can depend upon the output

achew22 · 2018-04-24T06:00:27Z

@f0rmiga I don't think you have to write the swagger rule any more 😉, great work @mrmeku!

f0rmiga · 2018-04-24T06:03:00Z

That is neat!

f0rmiga

Could you rename from protoc-gen-swagger.bzl to defs.bzl? Seems repetitive since it is already in the protoc-gen-swagger folder. Naming it defs.bzl is a good practice that folks have been using in other repos.

f0rmiga

Could you also format the .bzl file to use 4 spaces indentation and , at the end of each line in a list or multi-line parameter set?

f0rmiga · 2018-04-24T06:10:30Z

@achew22 I sent you a message about that in the Go Slack channel, but I guess you didn't see that.

achew22 · 2018-04-24T06:11:58Z

@f0rmiga, I mentioned you on the PR because I saw that message but I only got notified a few hours ago. Sorry if there was any overlapping work here.

f0rmiga · 2018-04-24T06:15:22Z

@achew22 No worries! No overlapping work here. Nice work @mrmeku!

mrmeku · 2018-04-24T06:16:10Z

@f0rmiga I think I'm done! Let me know if you have anything you can see that you'd like cleaned up

f0rmiga · 2018-04-24T06:16:59Z

third_party/googleapis/BUILD.bazel

+
+filegroup(
+    name = "googleapis",
+    srcs = glob(["**/*.proto"])


[nit] Missing ,.

f0rmiga · 2018-04-24T06:25:13Z

protoc-gen-swagger/defs.bzl

+            cfg = "host",
+        ),
+        "_googleapis": attr.label(
+            default = Label("@grpc_ecosystem_grpc_gateway//third_party/googleapis:googleapis"),


When I added the Bazel support initially I created the repositories.bzl that creates the @com_github_googleapis_googleapis repository target. The purpose of that repository target is to avoid using the .proto files under third_party. Could you try to use that repository target instead?

f0rmiga · 2018-04-24T06:29:10Z

protoc-gen-swagger/defs.bzl

+    args.add("--plugin=%s" % protoc_gen_swagger.path)
+    args.add("--swagger_out=logtostderr=true:%s" % swagger_file.dirname)
+    args.add("-Iexternal/com_google_protobuf/src")
+    args.add("-Iexternal/grpc_ecosystem_grpc_gateway/third_party/googleapis/")


It may be tricky to get the include directory for the protos from the repository target. Let me know if you have any questions regarding that.

f0rmiga · 2018-04-24T06:33:09Z

protoc-gen-swagger/defs.bzl

+
+protoc_gen_swagger = rule(
+    attrs = {
+        "proto_service": attr.label(


Instead of taking the .proto file, I think it would be better to take a proto_library so it is consistent on how the go_proto_library works. Take a look at the implementation here.

You could rename to proto instead of proto_service.

f0rmiga · 2018-04-24T06:35:49Z

protoc-gen-swagger/BUILD.bazel

@@ -19,6 +19,7 @@ go_library(
 go_binary(
    name = "protoc-gen-swagger",
    embed = [":go_default_library"],
+    visibility = ["//visibility:public"],


Not sure why you need to set the visibility to be public here.

This binary is now an implicit dependency of protoc_gen_swagger since it is listed as the default label for the _protoc_gen_swagger attribute of that rule. Since the protoc_gen_swagger rule is intended to be public, the dep itself needs to be publicly exposed

f0rmiga

You don't need third_party/googleapis/BUILD.bazel anymore.

mrmeku · 2018-04-24T06:41:37Z

Good point!

f0rmiga · 2018-04-24T06:50:25Z

examples/examplepb/BUILD.bazel

+
+protoc_gen_swagger(
+    name = "expamplepb_protoc_gen_swagger",
+    proto_service = "a_bit_of_everything.proto",


You need to change to proto = ":examplepb_proto",. If you do, then you will get a bunch of errors, because the output of the proto_library is a list of proto descriptors in .bin files. So you will need to use a Provider to grab the .proto files from the src list in the proto_library.

f0rmiga · 2018-04-24T06:52:27Z

protoc-gen-swagger/defs.bzl

+    attrs = {
+        "proto": attr.label(
+            mandatory = True,
+            allow_single_file = True,


You will need to remove this to accept a Label instead of files. From that Label you will extract the src files using a Provider.

f0rmiga · 2018-04-24T20:06:04Z

protoc-gen-swagger/defs.bzl

+
+protoc_gen_swagger = rule(
+    attrs = {
+        "deps": attr.label_list(


I still think this should be called proto, not deps. You also don't need it to be a label_list.

…braries

codecov-io · 2018-04-25T02:31:59Z

Codecov Report

Merging #613 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #613   +/-   ##
=======================================
  Coverage   59.28%   59.28%           
=======================================
  Files          28       28           
  Lines        2780     2780           
=======================================
  Hits         1648     1648           
  Misses        971      971           
  Partials      161      161

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 892952a...c5b759c. Read the comment docs.

f0rmiga · 2018-04-25T07:44:23Z

Good job @mrmeku! Thank you for your patience in the review. I appreciate it.

ensonic · 2018-04-27T10:38:06Z

Nice, is a support for proto options? E.g. #521 introduced: "grpc_api_configuration=path/to/your_api_config.yaml".

achew22 · 2018-04-27T13:57:20Z

No it isn't supported yet but it would be an easy pull request to add the functionality. Give it a go and send in whatever you can get working. I'll help you if you get stuck

ensonic · 2018-04-30T08:45:47Z

Sound good, I am on it. See #632

…ecosystem#613)

googlebot added the cla: yes label Apr 24, 2018

mrmeku force-pushed the proto_gen_swagger branch from b4553e8 to 83b7a7b Compare April 24, 2018 06:01

f0rmiga suggested changes Apr 24, 2018

View reviewed changes

f0rmiga reviewed Apr 24, 2018

View reviewed changes

mrmeku force-pushed the proto_gen_swagger branch from a20f0df to 1471505 Compare April 24, 2018 06:18

f0rmiga suggested changes Apr 24, 2018

View reviewed changes

f0rmiga reviewed Apr 24, 2018

View reviewed changes

f0rmiga suggested changes Apr 24, 2018

View reviewed changes

f0rmiga reviewed Apr 24, 2018

View reviewed changes

mrmeku force-pushed the proto_gen_swagger branch from c21f729 to e48a7ae Compare April 24, 2018 07:09

f0rmiga suggested changes Apr 24, 2018

View reviewed changes

mrmeku added 8 commits April 24, 2018 20:21

feat(bazel): Add rule for generating .swagger.json files

53f2eba

Rename protoc_gen_swagger.bzl to defs.bzl

8149796

Format defs.bzl

f573318

Insert missing comma

e871d21

Use com_github_googleapis_googleapis annotations.proto

c57e739

Remove third_party/googleapis/BUILD.bazel

31be8b9

Rename proto_service to proto

251ceca

Refactor protoc_gen_swagger to have only a deps attribute of proto_li…

9611c4a

…braries

mrmeku added 2 commits April 24, 2018 20:21

Remove need to depend on annotations.proto

c8a132c

Change deps to proto

c5b759c

mrmeku force-pushed the proto_gen_swagger branch from caddba1 to c5b759c Compare April 25, 2018 02:21

achew22 approved these changes Apr 25, 2018

View reviewed changes

achew22 merged commit 739cd2d into grpc-ecosystem:master Apr 25, 2018

adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020

feat(bazel): Add Bazel rule for generating .swagger.json files (grpc-…

65381c6

…ecosystem#613)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bazel): Add rule for generating .swagger.json files #613

feat(bazel): Add rule for generating .swagger.json files #613

mrmeku commented Apr 24, 2018

achew22 commented Apr 24, 2018

f0rmiga commented Apr 24, 2018

f0rmiga left a comment

f0rmiga left a comment

f0rmiga commented Apr 24, 2018

achew22 commented Apr 24, 2018

f0rmiga commented Apr 24, 2018

mrmeku commented Apr 24, 2018

f0rmiga Apr 24, 2018

mrmeku Apr 24, 2018

f0rmiga Apr 24, 2018

f0rmiga Apr 24, 2018

f0rmiga Apr 24, 2018

mrmeku Apr 24, 2018

f0rmiga Apr 24, 2018

f0rmiga Apr 24, 2018

mrmeku Apr 24, 2018 •

edited

Loading

f0rmiga left a comment

mrmeku commented Apr 24, 2018 •

edited

Loading

f0rmiga Apr 24, 2018

f0rmiga Apr 24, 2018

mrmeku Apr 24, 2018

f0rmiga Apr 24, 2018

codecov-io commented Apr 25, 2018

f0rmiga commented Apr 25, 2018

ensonic commented Apr 27, 2018

achew22 commented Apr 27, 2018

ensonic commented Apr 30, 2018 •

edited

Loading

feat(bazel): Add rule for generating .swagger.json files #613

feat(bazel): Add rule for generating .swagger.json files #613

Conversation

mrmeku commented Apr 24, 2018

achew22 commented Apr 24, 2018

f0rmiga commented Apr 24, 2018

f0rmiga left a comment

Choose a reason for hiding this comment

f0rmiga left a comment

Choose a reason for hiding this comment

f0rmiga commented Apr 24, 2018

achew22 commented Apr 24, 2018

f0rmiga commented Apr 24, 2018

mrmeku commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrmeku Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

f0rmiga left a comment

Choose a reason for hiding this comment

mrmeku commented Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Apr 25, 2018

Codecov Report

f0rmiga commented Apr 25, 2018

ensonic commented Apr 27, 2018

achew22 commented Apr 27, 2018

ensonic commented Apr 30, 2018 • edited Loading

mrmeku Apr 24, 2018 •

edited

Loading

mrmeku commented Apr 24, 2018 •

edited

Loading

ensonic commented Apr 30, 2018 •

edited

Loading