-
Notifications
You must be signed in to change notification settings - Fork 276
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
[suggestion] added plain text format#4218 #4236
Conversation
Signed-off-by: Nilav Prajapati <[email protected]>
Signed-off-by: Nilav Prajapati <[email protected]>
Signed-off-by: Nilav Prajapati <[email protected]>
Signed-off-by: Nilav Prajapati <[email protected]>
torii/src/routing.rs
Outdated
pub fn handle_health() -> Json { | ||
reply::json(&Health::Healthy) | ||
pub fn handle_health() -> impl warp::Reply { | ||
warp::reply::with_status("Healthy", StatusCode::OK).header("Content-Type", "text/plain") |
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.
Returning a &'static str
will be more terse and will still correctly set the Content-Type
header: https://docs.rs/warp/latest/src/warp/reply.rs.html#436-443
torii/src/routing.rs
Outdated
use iroha_version::Version; | ||
|
||
let string = sumeragi | ||
.apply_wsv(WorldStateView::latest_block_ref) | ||
.expect("Genesis not applied. Nothing we can do. Solve the issue and rerun.") | ||
.version() | ||
.to_string(); | ||
reply::json(&string) | ||
warp::reply::with_status(string, warp::http::StatusCode::OK).header("Content-Type", "text/plain") |
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.
Same, but can return the version String
directly
Also, please make sure your PR title and commit messages follow the format we use. This PR also should be a single commit (not four), as it is a small change. |
No problem ,I will do that by tomorrow |
@gerceboss will you finalize this PR? |
@gerceboss please comment :-) |
Description
Linked issue
Closes #4218
Benefits
removes redundancy
Checklist
CONTRIBUTING.md