Conversation
| let value = JsonValue::deserialize(deserializer)?; | ||
| let odata_type = value | ||
| .get("@odata.type") | ||
| .and_then(JsonValue::as_str) | ||
| .ok_or_else(|| serde::de::Error::missing_field("missing @odata.type in SSE payload"))?; | ||
|
|
||
| if odata_type.starts_with("#MetricReport.") { | ||
| let payload = | ||
| serde_json::from_value::<MetricReport>(value).map_err(serde::de::Error::custom)?; | ||
| Ok(Self::MetricReport(payload)) | ||
| } else if odata_type.starts_with("#Event.") { | ||
| let payload = | ||
| serde_json::from_value::<Event>(value).map_err(serde::de::Error::custom)?; | ||
| Ok(Self::Event(payload)) | ||
| } else { | ||
| Err(serde::de::Error::custom(format!( | ||
| "unsupported @odata.type in SSE payload: {odata_type}, should be either #Event.* or #MetricReport.*" | ||
| ))) | ||
| } |
There was a problem hiding this comment.
This parser is already available in core:
https://github.com/NVIDIA/nv-redfish/blob/main/core/src/odata.rs#L89
| /// `OData` identifier of the `EventService` in Redfish. | ||
| /// | ||
| /// Typically `/redfish/v1/EventService`. | ||
| #[must_use] | ||
| pub fn odata_id(&self) -> &ODataId { | ||
| self.data.id() | ||
| } |
There was a problem hiding this comment.
I think this is redundant. Who needs odata_id() and why raw().id() wouldn't work for somebody who needs it in rare occasion?
There was a problem hiding this comment.
It seems like a pattern for our resources, we have it in other mdoules
| let stream_id = ODataId::from(stream_uri.to_string()); | ||
| self.bmc | ||
| .as_ref() | ||
| .stream::<EventStreamPayload>(&stream_id) | ||
| .await | ||
| .map_err(Error::Bmc) | ||
| } |
There was a problem hiding this comment.
This conversion from URI to ODataId looks very suspicious. I believe that if URI is provided it should stay URI. Is there any reasons why Bmc::stream should have ODataId as parameter?
There was a problem hiding this comment.
URI provided as String, but it in practice this is ODataId
| /// `T` is structure that is used for the stream return type. | ||
| fn stream<T: Sized + for<'a> Deserialize<'a> + Send + 'static>( | ||
| &self, | ||
| id: &ODataId, |
There was a problem hiding this comment.
I commented this below but it looks like ODataId here is not the best choice. I would introduce another type like struct StreamPathRef<'a>(&String) and returned it from server_sent_event_uri function.
| let mock_server = MockServer::start().await; | ||
| let sse_body = concat!( | ||
| "event: Alert\n", | ||
| "data: {\"event_id\":\"1\",\"severity\":\"Critical\"}\n\n", |
There was a problem hiding this comment.
nit: Maybe better here: format!("data: {}", json!({event_id:"1","severity":"Critical"}))
There was a problem hiding this comment.
sse has wierd requiremetns on how it is formated, so this is easier to control (newlines are important).
| #[must_use] | ||
| pub fn server_sent_event_uri(&self) -> Result<&String, Error<B>> { | ||
| self.data | ||
| .server_sent_event_uri | ||
| .as_ref() | ||
| .ok_or(Error::EventServiceServerSentEventUriNotAvailable) | ||
| } |
There was a problem hiding this comment.
Maybe this function should stay private. We provide higher level interface for events access with events function.
2 participants
No description provided.