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

Remove REST Service from CLI #6408

Closed
4 tasks
alexanderbez opened this issue Jun 11, 2020 · 11 comments · Fixed by #6426
Closed
4 tasks

Remove REST Service from CLI #6408

alexanderbez opened this issue Jun 11, 2020 · 11 comments · Fixed by #6426

Comments

@alexanderbez
Copy link
Contributor

Summary

The aim of the CLI is interact and act as a proxy and command utility with a connected Cosmos-SDK based node running on Tendermint. From the CLI you can primarily create unsigned transactions, query the ledger state, and sign transactions.

There is the ad-hoc ability to also start a REST service (what was once known as the LCD). This is completely misplaced. Arguably, the CLI should not be responsible for this functionality.

Proposal

The REST service should be started with the binary (e.g. gaiad) on a separate port (could still be 1317). One this semantically makes more sense. Two, you may leverage certain functionality by belonging as part of the same process.

We may keep the wiring available for the REST service to be started via the CLI for a release to allow people to migrate or switch over.

/cc @aaronc @marbar3778 @jackzampolin


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@tac0turtle
Copy link
Member

How does this play into gRPC? I thought gRPC or REST would start with binary by default and the other you would need to manually enable.

@alexanderbez
Copy link
Contributor Author

alexanderbez commented Jun 11, 2020

Nothing changes on that front. All that is happening is the service is now started automatically in-process with the binary, instead of manually and separately.

gRPC or REST would start with binary by default

Exactly, gRPC and the API both start in-process.

and the other you would need to manually enable.

What is the other?

@alexanderbez alexanderbez added this to the v0.39 milestone Jun 11, 2020
@alexanderbez alexanderbez self-assigned this Jun 11, 2020
@jgimeno
Copy link
Contributor

jgimeno commented Jun 12, 2020

I totally support this, it is the natural way. I remember me searching in the past the flag to execute the REST service in gaiad to realize it was in the CLI.

Good one!

@fedekunze
Copy link
Collaborator

I fully support this as well. This was one of the main questions of a colleague of Chainsafe on why did we require to run the REST server as a separate process instead of the same one with the node.

@tac0turtle
Copy link
Member

Exactly, gRPC and the API both start in-process.

Ah okay I thought this was a given.

What is the other?

meant that if the gRPC process starts automatically then the REST would be optional. This may not be a worry but it may be beneficial to test the load of running both processes on the node.

@alexanderbez
Copy link
Contributor Author

@jgimeno if you have the bandwidth, this would be a fun issue to pick up! Let me know. If you don't, I take it on. Shouldn't be too hard. We can sync on it.

@jgimeno jgimeno self-assigned this Jun 12, 2020
@jgimeno
Copy link
Contributor

jgimeno commented Jun 12, 2020

I assign it to me also. :D

@aaronc
Copy link
Member

aaronc commented Jun 12, 2020

I would even go as far as to question why we have two binaries at all. All the cmd's could live in a single binary and it wouldn't be a problem. Currently 42mb for simcli and 50mb for simd, when the 50mb in simd has everything simcli has. Obviously beyond the scope of this issue, just making an observation...

Also another thing I'd eventually like to see is a bundled https://github.com/caddyserver/caddy server running in the same process for TLS/CORS. Again, just making a note.

@alexanderbez
Copy link
Contributor Author

Totally agree. Moving CLI over would be something I'd slate for 0.40. Doing the API service should be trivial, so we can do it for 0.39.

@tac0turtle
Copy link
Member

I would even go as far as to question why we have two binaries at all. All the cmd's could live in a single binary and it wouldn't be a problem. Currently 42mb for simcli and 50mb for simd, when the 50mb in simd has everything simcli has. Obviously beyond the scope of this issue, just making an observation...

image

@aaronc
Copy link
Member

aaronc commented Jun 12, 2020

I'll add issues for 0.40

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

Successfully merging a pull request may close this issue.

5 participants