gRPC-web proxy #8077

aleem1314 · 2020-12-04T07:11:08Z

closes: #7345

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

…eem/7345-grpc-web-proxy

codecov · 2020-12-04T07:20:09Z

Codecov Report

Merging #8077 (260276a) into master (334b223) will decrease coverage by 0.00%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master    #8077      +/-   ##
==========================================
- Coverage   61.91%   61.91%   -0.01%     
==========================================
  Files         609      610       +1     
  Lines       35100    35135      +35     
==========================================
+ Hits        21732    21753      +21     
- Misses      11085    11096      +11     
- Partials     2283     2286       +3

Impacted Files	Coverage Δ
server/config/toml.go	`18.18% <ø> (ø)`
server/config/config.go	`40.42% <50.00%> (+0.89%)`	⬆️
testutil/network/util.go	`71.02% <54.54%> (-1.89%)`	⬇️
server/grpc/grpc_web.go	`66.66% <66.66%> (ø)`
testutil/network/network.go	`89.95% <71.42%> (-0.59%)`	⬇️

…eem/7345-grpc-web-proxy

amaury1093

Nice work! Some initial questions about grpc-web-proxy.

Also: how hard would it be to add a test?

server/config/toml.go

amaury1093 · 2020-12-15T12:58:55Z

server/config/toml.go

+	# BindAddress defines address to bind the server to.
+	bind-address = "{{ .GRPC.GRPCWebProxy.BindAddress }}"
+
+	# HTTPPort defines TCP port to listen on for HTTP1.1 debug calls.
+	http-port = {{ .GRPC.GRPCWebProxy.HTTPPort }}


is it possible to bind to the same port as the grpc-gateway rest server?

server/config/toml.go

server/config/config.go

server/config/toml.go

server/grpc/grpcweb_proxy.go

server/start.go

…/cosmos-sdk into aleem/7345-grpc-web-proxy

anilcse

lgtm

server/start.go

Co-authored-by: Anil Kumar Kammari <anil@vitwit.com>

amaury1093

nice, thanks for the test!

my main question is the pros & cons of binding grpc-web on a separate port than the HTTP one.

server/config/config.go

amaury1093 · 2021-01-04T15:26:39Z

server/config/config.go

+	// Address defines the gRPC-web server to listen on
+	Address string `mapstructure:"address"`


We could also just keep the enable config, and if true, bind the server to the HTTP port. I'm not sure it's useful to expose so many ports. wdyt?

I tried to bind to the HTTP server. Getting this error.

I created this issue to discuss it: #8254

server/start.go

Co-authored-by: Amaury <amaury.martiny@protonmail.com>

amaury1093

lgtm

amaury1093 · 2021-01-11T13:54:17Z

@alexanderbez Since you requested changes in a previous iteration, could you have another look at this PR?

server/config/toml.go

server/start.go

testutil/network/network.go

server/start.go

* init * WIP config * WIP add proxy server * fmt * WIP * setup proxy server * clean go.mod * lint * lint * lint * custom codec * lint * add comments * change grpc-proxy port * add grpc-web * update server/start.go * add tests * add test with client * Update server/start.go Co-authored-by: Anil Kumar Kammari <anil@vitwit.com> * Update server/start.go Co-authored-by: Amaury <amaury.martiny@protonmail.com> * Update server/start.go Co-authored-by: Amaury <amaury.martiny@protonmail.com> * review changes * review changes * Update server/start.go Co-authored-by: Anil Kumar Kammari <anil@vitwit.com> Co-authored-by: Amaury <amaury.martiny@protonmail.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

aleem1314 added 3 commits December 3, 2020 19:34

init

6aa0309

Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into al…

93a277f

…eem/7345-grpc-web-proxy

WIP config

efaa0b4

aleem1314 added 13 commits December 4, 2020 17:53

WIP add proxy server

939eb79

fmt

f4f98eb

Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into al…

e3b7ab1

…eem/7345-grpc-web-proxy

WIP

758b4bb

setup proxy server

34c4eb6

Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into al…

e737111

…eem/7345-grpc-web-proxy

clean go.mod

8ff60e3

lint

d2cabcd

lint

a5d90bc

lint

dd6a000

custom codec

222d1cf

lint

a5e7dbf

Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into al…

34cbea8

…eem/7345-grpc-web-proxy

aleem1314 requested review from amaury1093 and anilcse December 14, 2020 07:04

aleem1314 added 3 commits December 15, 2020 14:33

add comments

e2e1704

change grpc-proxy port

3e1e495

Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into al…

95a71ac

…eem/7345-grpc-web-proxy

aleem1314 changed the title ~~WIP gRPC-web proxy~~ gRPC-web proxy Dec 15, 2020

aleem1314 marked this pull request as ready for review December 15, 2020 10:10

aleem1314 requested review from aaronc, alessio and alexanderbez as code owners December 15, 2020 10:10

amaury1093 reviewed Dec 15, 2020

View reviewed changes

alexanderbez suggested changes Dec 15, 2020

View reviewed changes

fedekunze added the C: gRPC Issues and PRs related to the gRPC service and HTTP gateway. label Dec 15, 2020

aleem1314 and others added 3 commits December 22, 2020 12:35

Merge branch 'master' into aleem/7345-grpc-web-proxy

c074ece

add test with client

d57513d

Merge branch 'aleem/7345-grpc-web-proxy' of https://github.com/cosmos…

4ffc71b

…/cosmos-sdk into aleem/7345-grpc-web-proxy

anilcse approved these changes Dec 23, 2020

View reviewed changes

server/start.go Outdated Show resolved Hide resolved

Update server/start.go

4debd21

Co-authored-by: Anil Kumar Kammari <anil@vitwit.com>

aleem1314 requested a review from alexanderbez December 23, 2020 15:32

Merge branch 'master' into aleem/7345-grpc-web-proxy

72f4aeb

amaury1093 reviewed Jan 4, 2021

View reviewed changes

aleem1314 and others added 3 commits January 4, 2021 22:09

Update server/start.go

b8bb16b

Co-authored-by: Amaury <amaury.martiny@protonmail.com>

Update server/start.go

f3bc9c6

Co-authored-by: Amaury <amaury.martiny@protonmail.com>

review changes

6e2f070

amaury1093 mentioned this pull request Jan 5, 2021

Should grpc-web be served on the HTTP port or on a separate port? #8254

Closed

4 tasks

amaury1093 approved these changes Jan 5, 2021

View reviewed changes

alexanderbez suggested changes Jan 11, 2021

View reviewed changes

server/config/toml.go Show resolved Hide resolved

server/start.go Outdated Show resolved Hide resolved

testutil/network/network.go Show resolved Hide resolved

review changes

5468ca0

aleem1314 requested a review from anilcse January 13, 2021 05:46

anilcse requested a review from alexanderbez January 13, 2021 17:09

Merge branch 'master' into aleem/7345-grpc-web-proxy

260276a

alexanderbez reviewed Jan 14, 2021

View reviewed changes

server/start.go Outdated Show resolved Hide resolved

alexanderbez approved these changes Jan 14, 2021

View reviewed changes

alexanderbez added 2 commits January 14, 2021 08:43

Merge branch 'master' into aleem/7345-grpc-web-proxy

9ba7192

Update server/start.go

24e8f9e

amaury1093 added the A:automerge Automatically merge PR once all prerequisites pass. label Jan 18, 2021

mergify bot added 2 commits January 18, 2021 17:00

Merge branch 'master' into aleem/7345-grpc-web-proxy

8b14c71

Merge branch 'master' into aleem/7345-grpc-web-proxy

c5e7fc6

mergify bot merged commit b771eff into master Jan 18, 2021

mergify bot deleted the aleem/7345-grpc-web-proxy branch January 18, 2021 18:25

iramiller mentioned this pull request Jul 19, 2021

Release v1.7.0 testing/verification provenance-io/provenance#379

Closed

38 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gRPC-web proxy #8077

gRPC-web proxy #8077

aleem1314 commented Dec 4, 2020

codecov bot commented Dec 4, 2020 •

edited

Loading

amaury1093 left a comment

amaury1093 Dec 15, 2020

anilcse left a comment

amaury1093 left a comment

amaury1093 Jan 4, 2021

aleem1314 Jan 4, 2021

amaury1093 Jan 5, 2021

amaury1093 left a comment

amaury1093 commented Jan 11, 2021

		// Address defines the gRPC-web server to listen on
		Address string `mapstructure:"address"`

gRPC-web proxy #8077

gRPC-web proxy #8077

Conversation

aleem1314 commented Dec 4, 2020

codecov bot commented Dec 4, 2020 • edited Loading

Codecov Report

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 Dec 15, 2020

Choose a reason for hiding this comment

anilcse left a comment

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 Jan 4, 2021

Choose a reason for hiding this comment

aleem1314 Jan 4, 2021

Choose a reason for hiding this comment

amaury1093 Jan 5, 2021

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 commented Jan 11, 2021

codecov bot commented Dec 4, 2020 •

edited

Loading