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 healthcheck endpoint to sqs #6935

Merged
merged 19 commits into from
Nov 29, 2023
Merged

Add healthcheck endpoint to sqs #6935

merged 19 commits into from
Nov 29, 2023

Conversation

mattverse
Copy link
Member

Closes: #XXX

What is the purpose of the change

Adds healthcheck endpoint that returns the following:

  • grpc Gateway status: Pings the status endpoint of the provided endpoint (address + port) returns down when status endpoint is unreachable
  • lastestHeight status: checks and returns if redis is synced to latest height of the chain
  • Redis status: checks and returns redis status

Testing and Verifying

WIP

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes?
  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

@mattverse mattverse marked this pull request as draft November 28, 2023 12:27
@github-actions github-actions bot added the C:app-wiring Changes to the app folder label Nov 28, 2023
Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

Great work! Left initial comments

ingest/sqs/system/delivery/http/system_http_handler.go Outdated Show resolved Hide resolved
ingest/sqs/system/delivery/http/system_http_handler.go Outdated Show resolved Hide resolved
ingest/sqs/system/delivery/http/system_http_handler.go Outdated Show resolved Hide resolved
Comment on lines +79 to +83
Result struct {
SyncInfo struct {
LatestBlockHeight string `json:"latest_block_height"`
} `json:"sync_info"`
} `json:"result"`
Copy link
Member

Choose a reason for hiding this comment

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

Is there some tendermint struct we can import instead of hardcoding these?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried changing this to import and use tendermint directly, the cometbft version we have imported and the actual tendermint version we are running on seems to have diference ( maybe due to diff between comet bft version between main and mainnet), retained this for now so that we can further test this

Copy link
Member

Choose a reason for hiding this comment

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

OK

@mattverse mattverse marked this pull request as ready for review November 29, 2023 10:44
@github-actions github-actions bot removed the C:x/mint label Nov 29, 2023
@mattverse
Copy link
Member Author

@p0mvn I have addressed all your prior comments except the one for using tendermint struct!

Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

Nice work sir

@p0mvn p0mvn merged commit 5826cb2 into roman/sqs-poc-v20 Nov 29, 2023
1 check passed
@p0mvn p0mvn deleted the mattverse/health branch November 29, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:app-wiring Changes to the app folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants