Skip to content

Commit f7f8c50

Browse files
fix
1 parent dca5bb6 commit f7f8c50

File tree

2 files changed

+159
-173
lines changed

2 files changed

+159
-173
lines changed

apps/framework-cli/src/framework/core/plan_validator.rs

Lines changed: 0 additions & 168 deletions
Original file line numberDiff line numberDiff line change
@@ -70,59 +70,12 @@ fn validate_cluster_references(project: &Project, plan: &InfraPlan) -> Result<()
7070
Ok(())
7171
}
7272

73-
/// Validates that replicated engines either have keeper path/replica name OR a cluster defined
74-
fn validate_replicated_engine_args(plan: &InfraPlan) -> Result<(), ValidationError> {
75-
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;
76-
77-
for table in plan.target_infra_map.tables.values() {
78-
let needs_args = match &table.engine {
79-
Some(ClickhouseEngine::ReplicatedMergeTree {
80-
keeper_path,
81-
replica_name,
82-
}) => keeper_path.is_none() && replica_name.is_none(),
83-
Some(ClickhouseEngine::ReplicatedReplacingMergeTree {
84-
keeper_path,
85-
replica_name,
86-
..
87-
}) => keeper_path.is_none() && replica_name.is_none(),
88-
Some(ClickhouseEngine::ReplicatedAggregatingMergeTree {
89-
keeper_path,
90-
replica_name,
91-
}) => keeper_path.is_none() && replica_name.is_none(),
92-
Some(ClickhouseEngine::ReplicatedSummingMergeTree {
93-
keeper_path,
94-
replica_name,
95-
..
96-
}) => keeper_path.is_none() && replica_name.is_none(),
97-
_ => false,
98-
};
99-
100-
// If engine args are missing AND no cluster is defined, that's an error
101-
if needs_args && table.cluster_name.is_none() {
102-
return Err(ValidationError::TableValidation(format!(
103-
"Table '{}' uses a replicated engine but neither cluster nor keeper path/replica name are specified.\n\
104-
\n\
105-
You must either:\n\
106-
1. Specify a cluster in the table config: cluster = \"prod_cluster\"\n\
107-
(and define it in moose.config.toml)\n\
108-
2. Or provide explicit keeper path and replica name in the engine config\n",
109-
table.name
110-
)));
111-
}
112-
}
113-
114-
Ok(())
115-
}
116-
11773
pub fn validate(project: &Project, plan: &InfraPlan) -> Result<(), ValidationError> {
11874
stream::validate_changes(project, &plan.changes.streaming_engine_changes)?;
11975

12076
// Validate cluster references
12177
validate_cluster_references(project, plan)?;
12278

123-
// Validate replicated engine args
124-
validate_replicated_engine_args(plan)?;
125-
12679
// Check for validation errors in OLAP changes
12780
for change in &plan.changes.olap_changes {
12881
if let OlapChange::Table(TableChange::ValidationError { message, .. }) = change {
@@ -393,127 +346,6 @@ mod tests {
393346
}
394347
}
395348

396-
#[test]
397-
fn test_replicated_engine_without_args_or_cluster_fails() {
398-
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;
399-
400-
let project = create_test_project(None);
401-
let table = create_table_with_engine(
402-
"test_table",
403-
None,
404-
Some(ClickhouseEngine::ReplicatedMergeTree {
405-
keeper_path: None,
406-
replica_name: None,
407-
}),
408-
);
409-
let plan = create_test_plan(vec![table]);
410-
411-
let result = validate(&project, &plan);
412-
413-
assert!(result.is_err());
414-
match result {
415-
Err(ValidationError::TableValidation(msg)) => {
416-
assert!(msg.contains("test_table"));
417-
assert!(msg.contains("replicated engine"));
418-
assert!(msg.contains("cluster"));
419-
}
420-
_ => panic!("Expected TableValidation error"),
421-
}
422-
}
423-
424-
#[test]
425-
fn test_replicated_engine_with_cluster_but_no_args_succeeds() {
426-
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;
427-
428-
let project = create_test_project(Some(vec![ClusterConfig {
429-
name: "prod_cluster".to_string(),
430-
}]));
431-
let table = create_table_with_engine(
432-
"test_table",
433-
Some("prod_cluster".to_string()),
434-
Some(ClickhouseEngine::ReplicatedMergeTree {
435-
keeper_path: None,
436-
replica_name: None,
437-
}),
438-
);
439-
let plan = create_test_plan(vec![table]);
440-
441-
let result = validate(&project, &plan);
442-
443-
assert!(result.is_ok());
444-
}
445-
446-
#[test]
447-
fn test_replicated_engine_with_args_but_no_cluster_succeeds() {
448-
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;
449-
450-
let project = create_test_project(None);
451-
let table = create_table_with_engine(
452-
"test_table",
453-
None,
454-
Some(ClickhouseEngine::ReplicatedMergeTree {
455-
keeper_path: Some("/clickhouse/tables/{database}/{table}".to_string()),
456-
replica_name: Some("{replica}".to_string()),
457-
}),
458-
);
459-
let plan = create_test_plan(vec![table]);
460-
461-
let result = validate(&project, &plan);
462-
463-
assert!(result.is_ok());
464-
}
465-
466-
#[test]
467-
fn test_replicated_engine_with_both_args_and_cluster_succeeds() {
468-
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;
469-
470-
let project = create_test_project(Some(vec![ClusterConfig {
471-
name: "prod_cluster".to_string(),
472-
}]));
473-
let table = create_table_with_engine(
474-
"test_table",
475-
Some("prod_cluster".to_string()),
476-
Some(ClickhouseEngine::ReplicatedMergeTree {
477-
keeper_path: Some("/clickhouse/tables/{database}/{table}".to_string()),
478-
replica_name: Some("{replica}".to_string()),
479-
}),
480-
);
481-
let plan = create_test_plan(vec![table]);
482-
483-
let result = validate(&project, &plan);
484-
485-
assert!(result.is_ok());
486-
}
487-
488-
#[test]
489-
fn test_replicated_replacing_merge_tree_without_args_or_cluster_fails() {
490-
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;
491-
492-
let project = create_test_project(None);
493-
let table = create_table_with_engine(
494-
"test_table",
495-
None,
496-
Some(ClickhouseEngine::ReplicatedReplacingMergeTree {
497-
keeper_path: None,
498-
replica_name: None,
499-
ver: Some("version".to_string()),
500-
is_deleted: None,
501-
}),
502-
);
503-
let plan = create_test_plan(vec![table]);
504-
505-
let result = validate(&project, &plan);
506-
507-
assert!(result.is_err());
508-
match result {
509-
Err(ValidationError::TableValidation(msg)) => {
510-
assert!(msg.contains("test_table"));
511-
assert!(msg.contains("replicated engine"));
512-
}
513-
_ => panic!("Expected TableValidation error"),
514-
}
515-
}
516-
517349
#[test]
518350
fn test_non_replicated_engine_without_cluster_succeeds() {
519351
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;

0 commit comments

Comments
 (0)