Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelorji committed Oct 17, 2023
1 parent f631df7 commit f860086
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 16 deletions.
14 changes: 7 additions & 7 deletions scylla-cql/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ pub enum QueryError {
#[error("Request timeout: {0}")]
RequestTimeout(String),

/// No known node found to perform query
#[error("No known node found: {0}")]
NoKnownNodeFoundError(String),
/// Empty Query Plan
#[error("Load balancing policy returned empty query plan. It can happen when the driver is provided with non-existing datacenter name")]
EmptyQueryPlan,

/// Address translation failed
#[error("Address translation failed: {0}")]
Expand Down Expand Up @@ -408,9 +408,9 @@ pub enum NewSessionError {
#[error("Client timeout: {0}")]
RequestTimeout(String),

/// No known node found to perform query
#[error("No known node found: {0}")]
NoKnownNodeFoundError(String),
/// Empty Query Plan
#[error("Load balancing policy returned empty query plan. It can happen when the driver is provided with non-existing datacenter name")]
EmptyQueryPlan,

/// Address translation failed
#[error("Address translation failed: {0}")]
Expand Down Expand Up @@ -490,7 +490,7 @@ impl From<QueryError> for NewSessionError {
QueryError::UnableToAllocStreamId => NewSessionError::UnableToAllocStreamId,
QueryError::RequestTimeout(msg) => NewSessionError::RequestTimeout(msg),
QueryError::TranslationError(e) => NewSessionError::TranslationError(e),
QueryError::NoKnownNodeFoundError(e) => NewSessionError::NoKnownNodeFoundError(e),
QueryError::EmptyQueryPlan => NewSessionError::EmptyQueryPlan,
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion scylla/src/transport/load_balancing/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2414,7 +2414,7 @@ mod latency_awareness {
| QueryError::IoError(_)
| QueryError::ProtocolError(_)
| QueryError::TimeoutError
| QueryError::NoKnownNodeFoundError(_)
| QueryError::EmptyQueryPlan
| QueryError::RequestTimeout(_) => true,
}
}
Expand Down
12 changes: 8 additions & 4 deletions scylla/src/transport/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1661,15 +1661,15 @@ impl Session {
QueryFut: Future<Output = Result<ResT, QueryError>>,
ResT: AllowedRunQueryResTType,
{
// set default error as no known found as the query plan returns an empty iterator if there are no nodes in the plan
let mut last_error: Option<QueryError> = Some(QueryError::NoKnownNodeFoundError(
"Please confirm the supplied datacenters exists".to_string(),
));
let mut last_error: Option<QueryError> = None;
let mut current_consistency: Consistency = context
.consistency_set_on_statement
.unwrap_or(execution_profile.consistency);

let mut query_plan_is_empty = true;

'nodes_in_plan: for node in query_plan {
query_plan_is_empty = false;
let span = trace_span!("Executing query", node = %node.address);
'same_node_retries: loop {
trace!(parent: &span, "Execution started");
Expand Down Expand Up @@ -1772,6 +1772,10 @@ impl Session {
}
}

if query_plan_is_empty {
return Some(Err(QueryError::EmptyQueryPlan));
}

last_error.map(Result::Err)
}

Expand Down
5 changes: 1 addition & 4 deletions scylla/src/transport/session_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2886,8 +2886,5 @@ async fn test_non_existent_dc_return_correct_error() {
let ks_stmt = format!("CREATE KEYSPACE IF NOT EXISTS {} WITH replication = {{'class': 'NetworkTopologyStrategy', '{}': 1}}", ks, dc);
let query_result = session.query(ks_stmt, &[]).await;

assert_matches!(
query_result.unwrap_err(),
QueryError::NoKnownNodeFoundError(_)
)
assert_matches!(query_result.unwrap_err(), QueryError::EmptyQueryPlan)
}

0 comments on commit f860086

Please sign in to comment.