WIP: jq rules engine refactor/tests #589
Conversation
|
|
Minimum allowed line rate is |
| pub fn get_event_id_from_method(method: Option<String>) -> Option<u64> { | ||
| method.and_then(|m| { | ||
| let event: Vec<&str> = m.split('.').collect(); | ||
| event.first().and_then(|v| v.parse::<u64>().ok()) |
There was a problem hiding this comment.
Add some safety for bad methods
There was a problem hiding this comment.
noted, will map/? it
| @@ -502,7 +540,7 @@ pub trait EndpointBroker { | |||
| } | |||
|
|
|||
| /// Generic method which takes the given parameters from RPC request and adds rules using rule engine | |||
| fn apply_request_rule(rpc_request: &BrokerRequest) -> Result<Value, RippleError> { | |||
| fn apply_request_rule(rpc_request: &BrokerRequest) -> Result<Value, JqError> { | |||
There was a problem hiding this comment.
Can we keep them as Ripple Error instead of JqError
Ripple error is already an enum which does the same for Deny Reason
There was a problem hiding this comment.
RippleError is overloaded imho, which is why i leaned into this more rust idiomatic apporach.
| pub fn run_broker_workflow( | ||
| broker_output: &BrokerOutput, | ||
| broker_request: &BrokerRequest, | ||
| ) -> Result<BrokerWorkflowSuccess, BrokerWorkFlowError> { |
There was a problem hiding this comment.
Same here lets keep RippleError instead of BrokerWorkFlowError and update the Enum
There was a problem hiding this comment.
Same as above, the goal of this approach is to provide more information, with a better chance of figuring out where the error was generated. What is the objective for using RippleError?
What
Defaultimplementation (both explicit and derived , where possible)Why
Automated testing of jq rules is currently not possible
Errors are obfuscated.
How
Refactor to expose appropriate components, more idiomatic use of rust' type system
Test
cargo testChecklist