Metrics api #466
Metrics api #466
Conversation
nomos-http-api/src/http/cl.rs
Outdated
| use std::hash::Hash; | ||
|
|
||
| #[derive(Clone, Debug, Serialize, Deserialize, Hash, PartialEq, Eq)] | ||
| pub struct Tx(pub String); |
There was a problem hiding this comment.
shouldn't we reuse the definitions from nomos-core?
There was a problem hiding this comment.
This Tx comes from nomos-node, I copied it from there. I am not sure if we want to use the Tx from nomos-core or do we want to make it generic over the Transaction trait in this service?
There was a problem hiding this comment.
This shouldn't go there, I don't like the idea that an http service defines the type of what's inside the mempool (while this was fine in a binary). Either we make it generic or reuse the definitions from nomos-core.
There was a problem hiding this comment.
uhm, At some point we used to have a custom Tx on nomos node binary if that is what @al8n refers. But yeah, the http-api should use generic types if possible. Otherwise just the types to be used by the nomos node itself. The second option should be ok imo as this service is completely ad-hoc to be used in the nomos binary. And no way types should be in this module/
…omos-node into http-api-axum-backend
yes, I think it is the case, in the same fashion as the new http-api. I would say we should move it. |
| #[cfg_attr( | ||
| feature = "openapi", | ||
| schema( | ||
| example = "e.g. [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]" |
There was a problem hiding this comment.
I think we talked about this, but can this be hex instead?
nodes/nomos-node-api/src/http/da.rs
Outdated
| handle: &overwatch_rs::overwatch::handle::OverwatchHandle, | ||
| items: Vec<<Blob as blob::Blob>::Hash>, | ||
| ) -> Result<Vec<Status>, super::DynError> { | ||
| let relay = handle.relay::<DaMempoolService>().connect().await.unwrap(); |
There was a problem hiding this comment.
We return an error here, let's not unwrap instead maybe?
| + Debug | ||
| + Hash | ||
| + Serialize | ||
| + for<'de> Deserialize<'de> |
There was a problem hiding this comment.
DeserializeOwned instead should be ok as well.
| let app = Router::new() | ||
| .merge(SwaggerUi::new("/swagger-ui").url("/api-docs/openapi.json", ApiDoc::openapi())) | ||
| .route("/da/metrics", routing::get(da_metrics)) | ||
| .route("/da/status", routing::post(routing::post(da_status))) |
There was a problem hiding this comment.
| .route("/da/status", routing::post(routing::post(da_status))) | |
| .route("/da/status", routing::post(da_status)) |
…into metrics-api
…into metrics-api
Add metrics-related API for new HTTP service.