Decouple StartHTTP{,AndTLS}Server from Listen()#2791
Decouple StartHTTP{,AndTLS}Server from Listen()#2791ebuchman merged 12 commits intotendermint:developfrom alessio:alessio/refactor-http-serve-calls
Conversation
This should help solve cosmos/cosmos-sdk#2715
tools/tm-monitor/rpc.go
Outdated
| if err != nil { | ||
| panic(err) | ||
| } | ||
| if err := rpc.StartHTTPServer(listener, mux, logger); err != nil { |
There was a problem hiding this comment.
We either need use a go-routine here or ensure that this is the last thing we call from the main method (ie. let it take the place of TrapSignal). In this case the Table-of-Nodes is created after startRPC, so we'd have to switch the order
ebuchman
left a comment
There was a problem hiding this comment.
Thanks for this - needs a few fixes.
tools/tm-monitor/rpc.go
Outdated
| if err != nil { | ||
| panic(err) | ||
| } | ||
| if err := rpc.StartHTTPServer(listener, mux, logger); err != nil { |
There was a problem hiding this comment.
We either need use a go-routine here or ensure that this is the last thing we call from the main method (ie. let it take the place of TrapSignal). In this case the Table-of-Nodes is created after startRPC, so we'd have to switch the order
|
Can we get develop merged in and add this as a BREAKING CHANGE to the GO API in the CHANGELOG_PENDING.md (should be v0.26.2). Thanks! |
Codecov Report
@@ Coverage Diff @@
## develop #2791 +/- ##
==========================================
- Coverage 62.33% 62.3% -0.04%
==========================================
Files 212 212
Lines 17262 17258 -4
==========================================
- Hits 10760 10752 -8
- Misses 5602 5606 +4
Partials 900 900
|
|
Non-deterministic failure, rerunning it. How should we handle |
|
@ebuchman we should have an |
Sounds right, but we can't just block on it. I guess it should be part of the shut-down sequence to catch that error. Only question is, if the error is from the startup phase, how do we catch it? Maybe we should open a new issue for this |
There was a problem hiding this comment.
Just need a bit more documentation around the change, otherwise looks good.
Thanks @alessio !
Question - I'd make the final changes myself but this branch is in your repo. Is there an easy way for me to do such a thing without having to fetch your branch and open a new PR ?
rpc/grpc/client_server.go
Outdated
| @@ -20,25 +17,10 @@ type Config struct { | |||
| // StartGRPCServer starts a new gRPC BroadcastAPIServer, listening on | |||
| // protoAddr, in a goroutine. Returns a listener and an error, if it fails to | |||
| // parse an address. | |||
There was a problem hiding this comment.
Update comment. Should also state that the function is blocking.
rpc/lib/server/http_server.go
Outdated
| ) | ||
|
|
||
| // StartHTTPServer starts an HTTP server on listenAddr with the given handler. | ||
| // StartHTTPServer takes a listener and start an HTTP server with the given handler. |
There was a problem hiding this comment.
Should mention that the function is blocking.
rpc/lib/server/http_server.go
Outdated
| // handler. | ||
|
|
||
|
|
||
| // StartHTTPAndTLSServer takes a listener and start an HTTP server with the given handler. |
There was a problem hiding this comment.
Mention the function is blocking.
|
@ebuchman dixit:
You take over here? You want me to do something else? |
| panic(err) | ||
| } | ||
| }() | ||
| listener2, err := server.Listen(unixAddr, server.Config{}) |
There was a problem hiding this comment.
| listener2, err := server.Listen(unixAddr, server.Config{}) | |
| listener2, err := server.Listen(unixAddr, server.Config{}) | |
| if err != nil { | |
| panic(err) | |
| } |
…mint#2791) (tendermint#2793) The cosmos-sdk 0.50 genesis doc marshalling not being compatible with the cometbft default one (see cosmos/cosmos-sdk#18477), so we need to support custom genesis doc provider in this public API as well, it's called by the `bootstrap-state` command in cosmos-sdk. <!-- Please add a reference to the issue that this PR addresses and indicate which files are most critical to review. If it fully addresses a particular issue, please include "Closes #XXX" (where "XXX" is the issue number). If this PR is non-trivial/large/complex, please ensure that you have either created an issue that the team's had a chance to respond to, or had some discussion with the team prior to submitting substantial pull requests. The team can be reached via GitHub Discussions or the Cosmos Network Discord server in the #cometbft channel. GitHub Discussions is preferred over Discord as it allows us to keep track of conversations topically. https://github.com/cometbft/cometbft/discussions If the work in this PR is not aligned with the team's current priorities, please be advised that it may take some time before it is merged - especially if it has not yet been discussed with the team. See the project board for the team's current priorities: https://github.com/orgs/cometbft/projects/1 --> --- #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments - [ ] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
This should help solve cosmos/cosmos-sdk#2715