-
Notifications
You must be signed in to change notification settings - Fork 97
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
Adds error reporting for CDC Mirror #335
Conversation
nexus/analyzer/src/lib.rs
Outdated
} | ||
|
||
fn stage_check(stage: String, peer_type: DbType)->Result<(), anyhow::Error>{ | ||
if stage.len() > 0 && !stage.starts_with("s3://") && peer_type == DbType::Snowflake { |
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.
warning: length comparison to zero
--> analyzer/src/lib.rs:39:8
|
39 | if stage.len() > 0 && !stage.starts_with("s3://") && peer_type == DbType::Snowflake {
| ^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!stage.is_empty()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
= note: `#[warn(clippy::len_zero)]` on by default
nexus/analyzer/src/lib.rs
Outdated
format!("{} {}", "cdc_staging_path", path_missing_err) | ||
)); | ||
} else if let Some(cdc_stage) = flow_job.cdc_staging_path.clone(){ | ||
let _ = stage_check(cdc_stage, destination_peer_type)?; |
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.
warning: this let-binding has unit value
--> analyzer/src/lib.rs:85:13
|
85 | let _ = stage_check(cdc_stage, destination_peer_type)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `stage_check(cdc_stage, destination_peer_type)?;`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
= note: `#[warn(clippy::let_unit_value)]` on by default
nexus/analyzer/src/lib.rs
Outdated
} | ||
|
||
fn stage_check(stage: String, peer_type: DbType) -> Result<(), anyhow::Error> { | ||
if stage.len() > 0 && !stage.starts_with("s3://") && peer_type == DbType::Snowflake { |
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.
warning: length comparison to zero
--> analyzer/src/lib.rs:39:8
|
39 | if stage.len() > 0 && !stage.starts_with("s3://") && peer_type == DbType::Snowflake {
| ^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!stage.is_empty()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
= note: `#[warn(clippy::len_zero)]` on by default
nexus/analyzer/src/lib.rs
Outdated
"cdc_staging_path", path_missing_err | ||
))); | ||
} else if let Some(cdc_stage) = flow_job.cdc_staging_path.clone() { | ||
let _ = stage_check(cdc_stage, destination_peer_type)?; |
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.
warning: this let-binding has unit value
--> analyzer/src/lib.rs:84:13
|
84 | let _ = stage_check(cdc_stage, destination_peer_type)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `stage_check(cdc_stage, destination_peer_type)?;`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
= note: `#[warn(clippy::let_unit_value)]` on by default
16bb18f
to
eb4d6f5
Compare
This PR adds error messages for missing or incorrect values of
snapshot/cdc_sync_mode
andsnapshot/cdc_staging_path
on the Nexus side.For ease of review, I've listed the error messages here:
Staging path for Snowflake must either be an S3 URL or an empty string for staging inside PeerDB.
snapshot/cdc_staging_path missing or invalid for your destination peer
(respectively)Tests have been added for these cases.