Updated change router to retain subscription after a pod restart #94
Conversation
ruokun-niu
requested review from
amansinghoriginal,
danielgerlag and
agentofreality
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| } | ||
| } | ||
| None => return Err(Box::from("State entry must be a JSON array")), |
There was a problem hiding this comment.
Why don't we just make the state_entry parameter a Vec<Value>?
There was a problem hiding this comment.
Good point. Also I checked Dapr's documentation and their version of save_state accepts the state_entry as Vec<u8>. Should we be consistent with them?
I guess the downside of using Vec is that we still need to deserialize the data into Value to check if the required fields are missing. Maybe it is better to use Vec for this
| for entry in entry { | ||
| match entry { | ||
| Value::Object(obj) => { | ||
| if !obj.contains_key("key") || !obj.contains_key("value") { |
There was a problem hiding this comment.
I think a strongly type object would be better here
|
|
||
| pub async fn save_state( | ||
| &self, | ||
| state_entry: Vec<Value>, |
There was a problem hiding this comment.
Could the API for the state manager take the strongly typed object, so that there is no need to validate for specific fields in an untyped object?
| let response = match dapr_client | ||
| .query_state_alpha1(config.clone().subscriber_store, query_condition, None) | ||
| .query_state_alpha1( |
There was a problem hiding this comment.
should we encapsulate both the reading and writing of state in the state_manager?
There was a problem hiding this comment.
Yeah that makes sense. Also we will have a deprovision function in the future for deleting the subscription from the state so it should also go into the same place
| } | ||
| }; | ||
| let publisher = DaprHttpPublisher::new( | ||
| "127.0.0.1".to_string(), |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
| config.pubsub_name.clone(), | ||
| topic, | ||
| ); | ||
| let state_manager = DaprStateManager::new("127.0.0.1", dapr_port, &config.subscriber_store); |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
| query_condition: Value, | ||
| metadata: Option<HashMap<String, String>>, | ||
| ) -> Result<Vec<Value>, Box<dyn std::error::Error>> { | ||
| let addr = "https://127.0.0.1".to_string(); |
Check notice
Code scanning / devskim
Accessing localhost could indicate debug code, or could hinder scaling. Note
Description
StateManagerthat is responsible for saving state values to the dapr state store. Thesave_statefunction from the Dapr Rust SDK will save the state as a serialized string, which prohibits thequery_state_alpha1function to fetch the state result.keyfield and avaluefieldType of change
Fixes: #issue_number