Conversation
dmidem
left a comment
There was a problem hiding this comment.
Looks good overall, I just have a few suggestions - mainly around replacing some instances of .unwrap() with .expect("...") .
zebrad/src/components/health.rs
Outdated
|
|
||
| async fn handle_request(req: Request<Incoming>) -> Result<Response<String>, Infallible> { | ||
| // Check if the request is for the health endpoint | ||
| if req.uri().path() != "/health" { |
There was a problem hiding this comment.
Should it additionally validate that it uses GET HTTP method only? Tracing endpoint validates that: https://github.com/QED-it/zebra/blob/zsa1/zebrad/src/components/tracing/endpoint.rs#L131
dmidem
left a comment
There was a problem hiding this comment.
CI fails now, to fix it, I suggest the following changes in Cargo.toml.
| insta = { version = "1.40.0", features = ["json"] } | ||
|
|
||
| # zebra-rpc needs the preserve_order feature, it also makes test results more stable | ||
| serde_json = { version = "1.0.132", features = ["preserve_order"] } |
There was a problem hiding this comment.
Consider restoring serde_json under [dev-dependencies] (while keeping it in [dependencies] with optional = true), because cargo test without --features=health-endpoint currently fails with unresolved imports in tests that use serde_json directly, e.g.:
error[E0432]: unresolved import `serde_json`
--> zebrad/tests/acceptance.rs:158:5
|
158 | use serde_json::Value;
| ^^^^^^^^^^ help: a similar path exists: `jsonrpc_core::serde_json`
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
Co-authored-by: Dmitry Demin <dmidem@users.noreply.github.com>
|
Closed in favor of |
Add health endpoint for Zebra node