Skip to content

Commit cfd29ca

Browse files
authored
feat(orchestration)!: orchestration names join the global unique-name check (#1394)
* feat(orchestration): jobs, groups, sensors and calendars join the global unique-name check Publication now rejects a job, job group, sensor or calendar schedule whose name is already used by any other project resource or orchestration declaration, with the same D007 conflict discovery raises for models, sources and the rest. The narrower duplicate-job and duplicate-trigger checks are removed; the object-identity check stays. build_orchestration_snapshot and compile_snapshot take the discovered project inputs so publication can build the full name set. Fixtures whose jobs reused model names are renamed, and DuckShop gains a name-collision edge case. * docs: sync the docs skill with the python nodes page * feat(orchestration): say how to rename a colliding declaration A name conflict already names both declarations and their files. It now ends with how to resolve it, and when a job, job group, sensor or calendar schedule is involved, says where that name comes from: a job, group or sensor takes it from its first argument, a calendar schedule from the function it decorates. * refactor(orchestration): collect declared names before scanning for conflicts The name check now gathers every declaration as a named record and then scans them once, instead of threading a mutable map through a helper. The first conflict found is the same.
1 parent b7a4f0b commit cfd29ca

36 files changed

Lines changed: 612 additions & 129 deletions

File tree

‎benchmarks/duckshop/edge_cases/orchestration/calendar_orders.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from sqlbuild.orchestration import calendar_schedule, job
44

5-
orders = job("orders", description="Build order reporting.", select="orders")
5+
orders = job("order_reporting", description="Build order reporting.", select="orders")
66

77

88
@calendar_schedule(

‎benchmarks/duckshop/edge_cases/orchestration/conflicting_settings.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from sqlbuild.orchestration import job
44

55
orders = job(
6-
"orders",
6+
"refresh_orders",
77
description="Refresh orders.",
88
select="orders",
99
settings={"load_sources": False, "reload": True},

‎benchmarks/duckshop/edge_cases/orchestration/invalid_settings.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from sqlbuild.orchestration import job
44

55
orders = job(
6-
"orders",
6+
"refresh_orders",
77
description="Refresh orders.",
88
select="orders",
99
settings={"select": "customers"},
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
"""A job may not reuse a model's name: jobs share the project's one resource namespace."""
2+
3+
from sqlbuild.orchestration import job
4+
5+
orders = job("orders", description="Build orders.", select="orders")

‎benchmarks/duckshop/edge_cases/orchestration/scheduler_crash_recovery.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
{
22
"description": "Three minutely cron jobs start ten hours back with the default latest catch-up. Two installed schedulers catch up while being SIGKILLed at seeded random moments; every lapse of a killed leader's lease is simulated. After a final uninterrupted scheduler, each trigger has ticked its latest due minute exactly once and nothing off schedule, each tick requests or is covered by exactly one run, and every requested run has one request event.",
33
"jobs": [
4-
"orders",
5-
"customers",
6-
"inventory"
4+
"refresh_orders",
5+
"refresh_customers",
6+
"refresh_inventory"
77
],
88
"backlog_minutes": 600,
99
"rounds": 16,
Lines changed: 104 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,149 @@
1-
//! Python's `_validate_unique_project_resource_names`: one namespace across resource kinds.
1+
//! One namespace across resource kinds, orchestration declarations included.
22
33
use std::collections::{HashMap, HashSet};
44

55
use crate::project_validation::_helpers::conflicts::{conflict, schema_seeds};
66
use crate::project_validation::_helpers::identities::block_or_file_name;
7-
use crate::project_validation::models::{NodeKind, ProjectFacts, ValidationFailure};
7+
use crate::project_validation::constants::ORCHESTRATION_RENAME_ADVICE;
8+
use crate::project_validation::models::{
9+
DeclaredName, NamedPath, NodeKind, ProjectFacts, ValidationFailure,
10+
};
811

912
/// Reject the first resource name declared twice across models, sources, seeds and the rest.
1013
pub(crate) fn validate_unique_project_resource_names(
1114
facts: &ProjectFacts,
1215
) -> Result<(), ValidationFailure> {
13-
let mut seen: HashMap<String, (String, String)> = HashMap::new();
14-
let mut declare = |name: &str, kind: &str, path: &str| -> Result<(), ValidationFailure> {
15-
if let Some((existing_kind, existing_path)) = seen.get(name) {
16-
return Err(conflict(format!(
17-
"Project resource name '{name}' is declared as both {existing_kind} in \
18-
{existing_path} and {kind} in {path}; model, source, seed, function, loader, \
19-
task, asset, check, provider, hook, unit test, and scenario names must be \
20-
globally unique"
21-
)));
16+
let seeds: Vec<NamedPath> = schema_seeds(facts);
17+
let mut seen: HashMap<&str, DeclaredName<'_>> = HashMap::new();
18+
for declared in declarations(facts, &seeds) {
19+
if let Some(first) = seen.get(declared.name) {
20+
return Err(name_conflict(first, &declared));
2221
}
23-
seen.insert(name.to_owned(), (kind.to_owned(), path.to_owned()));
24-
Ok(())
25-
};
22+
seen.insert(declared.name, declared);
23+
}
24+
Ok(())
25+
}
26+
27+
/// Every declared name in the order conflicts are reported.
28+
fn declarations<'a>(facts: &'a ProjectFacts, seeds: &'a [NamedPath]) -> Vec<DeclaredName<'a>> {
29+
let mut declared: Vec<DeclaredName<'a>> = Vec::new();
2630
for model in &facts.models {
27-
declare(&model.file.name, "model", &model.file.path)?;
31+
declared.push(DeclaredName {
32+
name: &model.file.name,
33+
kind: "model",
34+
path: &model.file.path,
35+
orchestration: false,
36+
});
2837
}
2938
let mut managed_sources: HashSet<&str> = HashSet::new();
3039
for file in &facts.sources {
3140
for entry in &file.entries {
3241
if entry.managed {
3342
managed_sources.insert(&entry.name);
3443
}
35-
declare(&entry.name, "source", &file.path)?;
44+
declared.push(DeclaredName {
45+
name: &entry.name,
46+
kind: "source",
47+
path: &file.path,
48+
orchestration: false,
49+
});
3650
}
3751
}
38-
for seed in schema_seeds(facts) {
39-
declare(&seed.name, "seed", &seed.path)?;
52+
for seed in seeds {
53+
declared.push(DeclaredName {
54+
name: &seed.name,
55+
kind: "seed",
56+
path: &seed.path,
57+
orchestration: false,
58+
});
4059
}
4160
for function in facts.sql_functions.iter().chain(&facts.python_functions) {
42-
declare(&function.name, "function", &function.path)?;
61+
declared.push(DeclaredName {
62+
name: &function.name,
63+
kind: "function",
64+
path: &function.path,
65+
orchestration: false,
66+
});
4367
}
4468
for node in &facts.nodes {
4569
if node.kind == NodeKind::Loader && managed_sources.contains(node.name.as_str()) {
4670
continue;
4771
}
48-
declare(&node.name, node.kind.label(), &node.path)?;
72+
declared.push(DeclaredName {
73+
name: &node.name,
74+
kind: node.kind.label(),
75+
path: &node.path,
76+
orchestration: false,
77+
});
4978
}
5079
for provider in &facts.providers {
51-
declare(&provider.name, "provider", &provider.path)?;
80+
declared.push(DeclaredName {
81+
name: &provider.name,
82+
kind: "provider",
83+
path: &provider.path,
84+
orchestration: false,
85+
});
5286
}
5387
for hook in &facts.sql_hooks {
54-
declare(&hook.name, "sql hook", &hook.path)?;
88+
declared.push(DeclaredName {
89+
name: &hook.name,
90+
kind: "sql hook",
91+
path: &hook.path,
92+
orchestration: false,
93+
});
5594
}
5695
for hook in &facts.hook_functions {
57-
declare(&hook.name, "python hook", &hook.path)?;
96+
declared.push(DeclaredName {
97+
name: &hook.name,
98+
kind: "python hook",
99+
path: &hook.path,
100+
orchestration: false,
101+
});
58102
}
59103
for scenario in &facts.scenarios {
60-
declare(&scenario.name, "scenario", &scenario.path)?;
104+
declared.push(DeclaredName {
105+
name: &scenario.name,
106+
kind: "scenario",
107+
path: &scenario.path,
108+
orchestration: false,
109+
});
61110
}
62111
let mut test_names: HashSet<&str> = HashSet::new();
63112
for test in &facts.tests {
64113
for block in &test.blocks {
65114
let name: &str = block_or_file_name(block.name.as_deref(), &test.file.name);
66-
if !test_names.insert(name) {
67-
continue;
115+
if test_names.insert(name) {
116+
declared.push(DeclaredName {
117+
name,
118+
kind: "unit test",
119+
path: &test.file.path,
120+
orchestration: false,
121+
});
68122
}
69-
declare(name, "unit test", &test.file.path)?;
70123
}
71124
}
72-
Ok(())
125+
for declaration in &facts.orchestration {
126+
declared.push(DeclaredName {
127+
name: &declaration.name,
128+
kind: declaration.kind,
129+
path: &declaration.path,
130+
orchestration: true,
131+
});
132+
}
133+
declared
134+
}
135+
136+
/// Name both declarations and say how to rename one of them.
137+
fn name_conflict(first: &DeclaredName<'_>, second: &DeclaredName<'_>) -> ValidationFailure {
138+
let advice: &str = if first.orchestration || second.orchestration {
139+
ORCHESTRATION_RENAME_ADVICE
140+
} else {
141+
""
142+
};
143+
conflict(format!(
144+
"Project resource name '{}' is declared as both {} in {} and {} in {}; model, source, seed, function, loader, task, asset, check, provider, \
145+
hook, unit test, scenario, job, job group, sensor, and calendar schedule names must be \
146+
globally unique; rename one of them{advice}",
147+
first.name, first.kind, first.path, second.kind, second.path
148+
))
73149
}

‎crates/sqlbuild-discovery/src/project_validation/constants.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,7 @@ pub(crate) const RESERVED_MODEL_NAMES: [&str; 1] = ["_chain_"];
1212
pub(crate) const INITIAL_SEED_READ_BYTES: u64 = 64 * 1024;
1313
/// A header and two data rows: enough to know a seed holds more than one data row.
1414
pub(crate) const RECORDS_FOR_MANY_ROWS: usize = 3;
15+
16+
/// How a colliding orchestration declaration is renamed, added to its name conflict.
17+
pub(crate) const ORCHESTRATION_RENAME_ADVICE: &str = " (a job, job group or sensor takes its name \
18+
from its first argument, and a calendar schedule from the function it decorates)";
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
pub mod at_most_one_csv_data_row;
2+
pub mod validate_orchestration_names;
23
pub mod validate_project_inputs;
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//! Orchestration declarations share the project's one resource namespace.
2+
3+
use crate::project_validation::_helpers::resource_names::validate_unique_project_resource_names;
4+
use crate::project_validation::models::{ProjectFacts, ValidationFailure};
5+
6+
/// Reject the first name a job, group, sensor or calendar schedule shares with anything else.
7+
pub fn validate_orchestration_names(facts: &ProjectFacts) -> Result<(), ValidationFailure> {
8+
validate_unique_project_resource_names(facts)
9+
}

‎crates/sqlbuild-discovery/src/project_validation/models.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,26 @@ pub struct ProjectFacts {
178178
pub project_model_paths: Option<Vec<String>>,
179179
/// Python's `csv.field_size_limit()`, or `None` for its default.
180180
pub csv_field_limit: Option<usize>,
181+
/// Jobs, job groups, sensors and calendar schedules, known only when a location publishes.
182+
pub orchestration: Vec<OrchestrationNameFact>,
183+
}
184+
185+
/// One name in the project's resource namespace, as a name conflict reports it.
186+
#[derive(Debug)]
187+
pub(crate) struct DeclaredName<'a> {
188+
pub(crate) name: &'a str,
189+
pub(crate) kind: &'a str,
190+
pub(crate) path: &'a str,
191+
/// A job, job group, sensor or calendar schedule, whose conflicts say how it is named.
192+
pub(crate) orchestration: bool,
193+
}
194+
195+
/// A named orchestration declaration: its name, its kind as conflicts word it, and its module.
196+
#[derive(Clone, Debug, PartialEq, Eq)]
197+
pub struct OrchestrationNameFact {
198+
pub name: String,
199+
pub kind: &'static str,
200+
pub path: String,
181201
}
182202

183203
/// Why discovered inputs are rejected: a discovery error, or a seed CSV Python cannot read.

0 commit comments

Comments
 (0)