Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Add query-only mode flag #1360

Merged
merged 17 commits into from
Oct 19, 2022
Merged

Add query-only mode flag #1360

merged 17 commits into from
Oct 19, 2022

Conversation

mmsqe
Copy link
Contributor

@mmsqe mmsqe commented Sep 28, 2022

Closes: #1359

Description


For contributor use:

  • 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

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@yihuang
Copy link
Contributor

yihuang commented Sep 28, 2022

In the context of ethermint, probabely we need to have a mode that start grpc and json-rpc together, but could leave that to another issue.

@mmsqe mmsqe marked this pull request as ready for review September 28, 2022 07:37
@mmsqe mmsqe requested a review from a team as a code owner September 28, 2022 07:37
@mmsqe mmsqe requested review from 0a1c and adisaran64 and removed request for a team September 28, 2022 07:37
@mmsqe
Copy link
Contributor Author

mmsqe commented Sep 29, 2022

In the context of ethermint, probabely we need to have a mode that start grpc and json-rpc together, but could leave that to another issue.

maybe just query-only flag, we can pass like grpc|jsonrpc|grpc,jsonrpc

@mmsqe mmsqe changed the title Add grpc only mode flag Add query-only mode flag Sep 29, 2022
server/start.go Outdated Show resolved Hide resolved
@yihuang
Copy link
Contributor

yihuang commented Sep 29, 2022

In the context of ethermint, probabely we need to have a mode that start grpc and json-rpc together, but could leave that to another issue.

maybe just query-only flag, we can pass like grpc|jsonrpc|grpc,jsonrpc

or --json-rpc-only which implies all the dependency services.

Copy link
Contributor

@adisaran64 adisaran64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think it might be a good idea to use a bool flag instead of a string for the query-only mode, seems to simplify the code and use?

server/start.go Outdated Show resolved Hide resolved
server/start.go Outdated Show resolved Hide resolved
server/start.go Outdated Show resolved Hide resolved
server/start.go Outdated Show resolved Hide resolved
server/start.go Outdated Show resolved Hide resolved
@mmsqe mmsqe requested review from adisaran64 and removed request for 0a1c September 29, 2022 13:17
server/start.go Fixed Show fixed Hide fixed
@mmsqe mmsqe force-pushed the grpc_only branch 2 times, most recently from 5953248 to 06b7d82 Compare September 29, 2022 13:23
server/start.go Fixed Show fixed Hide fixed
@fedekunze fedekunze requested review from facs95, danburck, v-homsi and a team October 19, 2022 07:34
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the JSON-RPC only flag will work properly without tendermint. The --grpc-only looks good to me

server/flags/flags.go Outdated Show resolved Hide resolved
server/start.go Outdated Show resolved Hide resolved
@fedekunze fedekunze merged commit 236ca33 into evmos:main Oct 19, 2022
@danburck danburck mentioned this pull request Nov 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cosmos-sdk --grpc-only is not integrated in start cmd
5 participants