Skip to content
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

Closed
wants to merge 4 commits into from
Closed

Conversation

Amogh-Bharadwaj
Copy link
Contributor

This PR adds error messages for missing or incorrect values of snapshot/cdc_sync_mode and snapshot/cdc_staging_path on the Nexus side.

For ease of review, I've listed the error messages here:

  1. For Snowflake as destination peer of mirror: Staging path for Snowflake must either be an S3 URL or an empty string for staging inside PeerDB.
  2. snapshot/cdc_staging_path missing or invalid for your destination peer (respectively)

Tests have been added for these cases.

}

fn stage_check(stage: String, peer_type: DbType)->Result<(), anyhow::Error>{
if stage.len() > 0 && !stage.starts_with("s3://") && peer_type == DbType::Snowflake {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [clippy] reported by reviewdog 🐶

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

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)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [clippy] reported by reviewdog 🐶

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

}

fn stage_check(stage: String, peer_type: DbType) -> Result<(), anyhow::Error> {
if stage.len() > 0 && !stage.starts_with("s3://") && peer_type == DbType::Snowflake {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [clippy] reported by reviewdog 🐶

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

"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)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [clippy] reported by reviewdog 🐶

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

@serprex serprex deleted the better-error branch July 19, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants