-
Notifications
You must be signed in to change notification settings - Fork 972
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
feat(rpc/node): readiness check #3118
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3118 +/- ##
==========================================
+ Coverage 51.97% 52.01% +0.03%
==========================================
Files 178 178
Lines 11239 11239
==========================================
+ Hits 5842 5846 +4
+ Misses 4899 4898 -1
+ Partials 498 495 -3 ☔ View full report in Codecov by Sentry. |
arguably fixes #739 |
@distractedm1nd read perms IMO, no public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a test imo
will add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI needs a rerun
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine w me, utack on the RPC being started last. Does moving the construction of the rpc mod last affect the appends to the start hooks?
Why is this breaking @distractedm1nd ? |
adds new field to config, ah so not really breaking ig |
@ramin I'd argue doesn't solve need for readiness endpoint because it's a json-rpc. HTTP probes in k8s are exclusively on GET, and JSON-RPC is exclusively post: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#http-probes |
@joroshiba was the original convo / request for a non-rpc readiness endpoint and not this? |
should this be documented? if so, how does one use it? |
Implements a readiness check on the node module.
Question: I have set the permission level to public, but remember that we removed all public methods in favor of
read
. Is this fine or should this method also beread
?Comes from a conversation with @joroshiba from Astria