Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add base_path as constant in generated gateway files #1650

Closed
lelvisl opened this issue Sep 5, 2020 · 5 comments
Closed

Add base_path as constant in generated gateway files #1650

lelvisl opened this issue Sep 5, 2020 · 5 comments

Comments

@lelvisl
Copy link

lelvisl commented Sep 5, 2020

🚀 Feature

Add base_path as constant in generated gateway files will be a good feature.
I try to mount two grpc services on one http router

	schema.RegisterStorageServer(grpcServer, apiSvc)
	schema.RegisterExternalServiceServer(grpcServer, apiSvc.ExternalService)
	schema.RegisterDocumentsServiceServer(grpcServer, apiSvc.DocumentsService)
	externalGateway := runtime.NewServeMux(runtime.WithMarshalerOption(runtime.MIMEWildcard, &runtime.JSONPb{OrigName: true, EmitDefaults: true}))
	documentsGateway := runtime.NewServeMux(runtime.WithMarshalerOption(runtime.MIMEWildcard, &runtime.JSONPb{OrigName: true, EmitDefaults: true}))

	err = schema.RegisterExternalServiceHandlerServer(ctx, externalGateway, apiSvc.ExternalService)
	err = schema.RegisterDocumentsServiceHandlerServer(ctx, documentsGateway, apiSvc.DocumentsService)
	externalBasePath:="/api/external/v1"
	documentsBasePath:="/api/external/v1"

	router := chi.NewRouter()
	router.Mount(externalBasePath, http.StripPrefix(externalBasePath, externalGateway))
	router.Mount(documentsBasePath, http.StripPrefix(documentsBasePath, externalGateway))

If externalBasePath and documentsBasePath are in the generated files - it will be very useful.

@johanbrandhorst
Copy link
Collaborator

What would be the source of truth for this variable? The existing swagger annotation is not used by the gateway generator and we're unlikely to change that just for this feature.

@lelvisl
Copy link
Author

lelvisl commented Sep 5, 2020

yes, i thought about annotation in protofile (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger).

we're unlikely to change that just for this feature.

why?

@johanbrandhorst
Copy link
Collaborator

It will be harder than you think to add this, and I don't think we want to add the maintenance cost of supporting both the http annotations and swagger annotations in the gateway generator. The swagger annotations affect only the swagger output, and not the output of the other generator, and that keeps things simpler. This feature is not important enough for us to change that. Sorry.

@lelvisl
Copy link
Author

lelvisl commented Sep 5, 2020

if i implement it myself - will you accept this code?

@johanbrandhorst
Copy link
Collaborator

No, because it's us maintainers who have to maintain the code. I don't think this feature is important enough to;

  1. Require protoc-gen-grpc-gateway to know about the swagger annotations
  2. To break with the current practice of not having the swagger annotations affect the generated Go code, e.g. it is only for modifying the generated OpenAPI spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants