-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(routing): Add support to update config for elimination routing #7938
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
Conversation
Changed Files
|
…gle-elimination-routing
crates/router/src/core/routing.rs
Outdated
algorithm_id: common_utils::id_type::RoutingId, | ||
profile_id: common_utils::id_type::ProfileId, | ||
) -> RouterResponse<routing_types::RoutingDictionaryRecord> { | ||
use external_services::grpc_client::dynamic_routing::elimination_based_client::EliminationBasedRouting; |
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.
move this to the top
crates/router/src/core/routing.rs
Outdated
metrics::ROUTING_UPDATE_CONFIG_FOR_PROFILE.add( | ||
1, | ||
router_env::metric_attributes!(("profile_id", profile_id.clone())), | ||
); |
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.
can we add algo_type as attribute to get the count of updates of specific algo
cc: @prajjwalkumar17
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.
Yes can be made as an attribute, but the RoutingAlgorithmKind
we use in RoutingAlgorithm
table has type as DyanamicAlgorithm
, we can create an enum something like DynamicRoutingAlgorithmKind
which will have types as SuccessBasedRoutingAlgorithm
, EliminationAlgorithm
, ContractRoutingAlgorithm
and this can be used only to fill the respective metric's attributes in the respective function calls, no DB changes required only assign the enum in function and add to metrics call, this needs to be done for all the dynamic routing functions for create as well as update.
cc: @spritianeja03
crates/router/src/core/routing.rs
Outdated
let _ = cache::redact_from_redis_and_publish( | ||
state.store.get_cache_store().as_ref(), | ||
cache_entries_to_redact, | ||
) | ||
.await | ||
.map_err(|e| logger::error!("unable to publish into the redact channel for evicting the elimination routing config cache {e:?}")); |
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.
let _ = cache::redact_from_redis_and_publish( | |
state.store.get_cache_store().as_ref(), | |
cache_entries_to_redact, | |
) | |
.await | |
.map_err(|e| logger::error!("unable to publish into the redact channel for evicting the elimination routing config cache {e:?}")); | |
cache::redact_from_redis_and_publish( | |
state.store.get_cache_store().as_ref(), | |
cache_entries_to_redact, | |
) | |
.await | |
.map_err(|e| logger::error!("unable to publish into the redact channel for evicting the elimination routing config cache {e:?}")).ok(); |
crates/router/src/core/routing.rs
Outdated
) | ||
.await | ||
.change_context(errors::ApiErrorResponse::GenericNotFoundError { | ||
message: "Failed to invalidate the routing keys".to_string(), |
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.
message: "Failed to invalidate the routing keys".to_string(), | |
message: "Failed to invalidate the elimination routing keys".to_string(), |
crates/router/src/core/routing.rs
Outdated
state.get_grpc_headers(), | ||
) | ||
.await | ||
.change_context(errors::ApiErrorResponse::GenericNotFoundError { |
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.
This is not GenericNotFoundError error. all other algo's update handler too has this error mapped to GenericNotFoundError. please change it in other update flows as well
…gle-elimination-routing
Type of Change
Description
Add a new route for updating the elimination routing config when it is toggled
Additional Changes
Motivation and Context
This PR closes #7939
How did you test it?
request
response
request
response
Checklist
cargo +nightly fmt --all
cargo clippy