From 7948c79fa1ab0ba35634cd280280e5830003c229 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Sun, 15 Nov 2026 00:42:30 +0000 Subject: [PATCH] feat: add a named-selector limit Rule for orchestration jobs and sensors SQBRORCHESTRATION101 reports a job select, or the select of a node_event, source_new_data or source_stale sensor condition, that names more than max_named_selectors nodes. Sensor conditions are followed through module-level names and imports from project modules; what cannot be resolved is reported and can be suppressed with an exact exception. --- .../models/orders/int_orders.sql | 2 + .../models/orders/order_totals.sql | 2 + .../models/orders/stg_orders.sql | 2 + .../models/orders/stg_payments.sql | 2 + .../orchestration/jobs/orders_folder_job.toml | 4 + .../orchestration/jobs/orders_listed_job.toml | 4 + .../orchestration/jobs/orders_repair_job.toml | 4 + .../sensors/_orders_conditions.py | 14 + .../sensors/on_orders_built_sensor.py | 12 + .../sensors/on_orders_computed_sensor.py | 12 + .../sensors/on_orders_failure_sensor.py | 14 + .../sensors/on_orders_landed_sensor.py | 12 + .../sensors/on_tick_overdue_sensor.py | 12 + .../sqlbuild-analysis/src/graph/main/mod.rs | 1 + .../src/graph/main/named_selector_atoms.rs | 28 ++ crates/sqlbuild-analysis/src/graph/tests.rs | 2 + .../graph/tests/test_named_selector_atoms.rs | 66 +++++ .../src/graph/tests/test_types.rs | 6 + .../_helpers/python_modules.rs | 1 + .../src/orchestration_files/models.rs | 4 + .../orchestration_modules/_helpers/calls.rs | 17 +- .../_helpers/conditions.rs | 179 ++++++++++++ .../_helpers/declarations.rs | 112 ++++++++ .../src/orchestration_modules/_helpers/mod.rs | 2 + .../src/orchestration_modules/constants.rs | 11 + .../src/orchestration_modules/main/mod.rs | 1 + .../main/module_binding_condition.rs | 51 ++++ .../main/orchestration_module_declarations.rs | 69 +---- .../src/orchestration_modules/models.rs | 50 ++++ .../orchestration_modules/tests/helpers.rs | 1 + .../test_orchestration_module_declarations.rs | 112 +++++++- crates/sqlbuild-rules/src/constants.rs | 2 + .../src/engine/_helpers/evaluation.rs | 25 +- .../src/engine/_helpers/memo.rs | 2 + .../engine/tests/test_incremental_rules.rs | 1 + crates/sqlbuild-rules/src/lib.rs | 1 + crates/sqlbuild-rules/src/models.rs | 4 + .../orchestration_rules/_helpers/imports.rs | 116 ++++++++ .../src/orchestration_rules/_helpers/mod.rs | 2 + .../_helpers/selections.rs | 95 +++++++ .../src/orchestration_rules/constants.rs | 16 ++ .../main/evaluate_orchestration_rules.rs | 94 +++++++ .../src/orchestration_rules/main/mod.rs | 2 + .../main/orchestration_facts.rs | 31 +++ .../src/orchestration_rules/mod.rs | 9 + .../src/orchestration_rules/models.rs | 27 ++ .../src/orchestration_rules/tests.rs | 6 + .../src/orchestration_rules/tests/helpers.rs | 67 +++++ .../tests/test_named_selector_limit.rs | 257 ++++++++++++++++++ .../orchestration_rules/tests/test_types.rs | 6 + .../src/python_rules/main/python_sources.rs | 11 +- .../src/rules/_helpers/catalogue.rs | 9 + .../src/rules/main/builtin_options.rs | 8 + .../src/rules/main/evaluate_project.rs | 5 + crates/sqlbuild-rules/src/rules/main/mod.rs | 1 + .../src/rules/main/request_selects.rs | 17 ++ .../sqlbuild/references/docs/CONTENTS.md | 1 + .../references/docs/concepts/orchestration.md | 4 + .../references/docs/concepts/rules.md | 4 +- .../rules/configuration-and-selection.md | 3 + .../concepts/rules/orchestration-rules.md | 95 +++++++ .../sqlbuild/cli/commands/main/_test_types.py | 11 + .../src/sqlbuild/cli/commands/main/helpers.py | 13 + .../sqlbuild/cli/commands/main/test_rules.py | 128 +++++++++ .../_helpers/engine/test_catalogue.py | 2 + .../docs/docs/concepts/orchestration.mdx | 4 + .../src/content/docs/docs/concepts/rules.mdx | 4 +- .../rules/configuration-and-selection.mdx | 3 + .../concepts/rules/orchestration-rules.mdx | 92 +++++++ website/src/sidebar.json | 1 + 70 files changed, 1903 insertions(+), 85 deletions(-) create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/models/orders/int_orders.sql create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/models/orders/order_totals.sql create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_orders.sql create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_payments.sql create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_folder_job.toml create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_listed_job.toml create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_repair_job.toml create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/_orders_conditions.py create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_built_sensor.py create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_computed_sensor.py create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_failure_sensor.py create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_landed_sensor.py create mode 100644 benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_tick_overdue_sensor.py create mode 100644 crates/sqlbuild-analysis/src/graph/main/named_selector_atoms.rs create mode 100644 crates/sqlbuild-analysis/src/graph/tests/test_named_selector_atoms.rs create mode 100644 crates/sqlbuild-pysource/src/orchestration_modules/_helpers/conditions.rs create mode 100644 crates/sqlbuild-pysource/src/orchestration_modules/_helpers/declarations.rs create mode 100644 crates/sqlbuild-pysource/src/orchestration_modules/main/module_binding_condition.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/_helpers/imports.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/_helpers/mod.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/_helpers/selections.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/constants.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/main/evaluate_orchestration_rules.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/main/mod.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/main/orchestration_facts.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/mod.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/models.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/tests.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/tests/helpers.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/tests/test_named_selector_limit.rs create mode 100644 crates/sqlbuild-rules/src/orchestration_rules/tests/test_types.rs create mode 100644 crates/sqlbuild-rules/src/rules/main/request_selects.rs create mode 100644 src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/orchestration-rules.md create mode 100644 website/src/content/docs/docs/concepts/rules/orchestration-rules.mdx diff --git a/benchmarks/duckshop/edge_cases/named_selectors/models/orders/int_orders.sql b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/int_orders.sql new file mode 100644 index 000000000..7e9bcbef0 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/int_orders.sql @@ -0,0 +1,2 @@ +MODEL (description "Synthetic orders edge case int_orders"); +SELECT 1 AS order_id diff --git a/benchmarks/duckshop/edge_cases/named_selectors/models/orders/order_totals.sql b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/order_totals.sql new file mode 100644 index 000000000..95527d44d --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/order_totals.sql @@ -0,0 +1,2 @@ +MODEL (description "Synthetic orders edge case order_totals"); +SELECT 1 AS order_id diff --git a/benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_orders.sql b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_orders.sql new file mode 100644 index 000000000..bf4ca03ea --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_orders.sql @@ -0,0 +1,2 @@ +MODEL (description "Synthetic orders edge case stg_orders"); +SELECT 1 AS order_id diff --git a/benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_payments.sql b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_payments.sql new file mode 100644 index 000000000..592024c10 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/models/orders/stg_payments.sql @@ -0,0 +1,2 @@ +MODEL (description "Synthetic orders edge case stg_payments"); +SELECT 1 AS order_id diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_folder_job.toml b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_folder_job.toml new file mode 100644 index 000000000..934ac24aa --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_folder_job.toml @@ -0,0 +1,4 @@ +# A folder, an expansion, a glob, an intersection's name, a job name and a repeated name: one named node. +name = "orders_folder_job" +description = "Selects the orders pipeline without listing it." +select = "models/orders +order_totals stg_* order_totals,tag:orders orders_listed_job order_totals" diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_listed_job.toml b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_listed_job.toml new file mode 100644 index 000000000..0c3341f96 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_listed_job.toml @@ -0,0 +1,4 @@ +# Four nodes listed one by one: over the default limit of three. +name = "orders_listed_job" +description = "Lists every orders model by name." +select = "stg_orders stg_payments int_orders order_totals" diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_repair_job.toml b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_repair_job.toml new file mode 100644 index 000000000..012a569cd --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/jobs/orders_repair_job.toml @@ -0,0 +1,4 @@ +# A deliberate list, kept by an exact rule exception in the test's [rules] table. +name = "orders_repair_job" +description = "Rebuilds the four orders models after a late payments load." +select = "stg_orders stg_payments int_orders order_totals" diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/_orders_conditions.py b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/_orders_conditions.py new file mode 100644 index 000000000..331333c98 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/_orders_conditions.py @@ -0,0 +1,14 @@ +"""Conditions the orders sensors import: one over the limit, a platform event and a builder.""" + +from sqlbuild.orchestration import PlatformEventKind, node_event, platform_event + +ORDERS_LANDED = node_event( + select="stg_orders stg_payments int_orders order_totals", kinds=["success"] +) +TICK_OVERDUE = platform_event(kinds=[PlatformEventKind.TICK_OVERDUE]) + + +def landed_after(model: str) -> object: + """A success condition on one model, built when the sensor module loads.""" + + return node_event(select=model, kinds=["success"]) diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_built_sensor.py b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_built_sensor.py new file mode 100644 index 000000000..5d1d4ca3b --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_built_sensor.py @@ -0,0 +1,12 @@ +"""Refreshes totals after one model: a condition a function builds, which cannot be counted.""" + +from orchestration.sensors._orders_conditions import landed_after + +from sqlbuild.orchestration import sensor + +on_orders_built = sensor( + "on_orders_built_sensor", + description="Refresh order totals after the totals model", + when=landed_after("order_totals"), + run="orders_folder_job", +) diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_computed_sensor.py b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_computed_sensor.py new file mode 100644 index 000000000..21e65b12d --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_computed_sensor.py @@ -0,0 +1,12 @@ +"""A condition whose select is not a string literal cannot be counted and is reported.""" + +from sqlbuild.orchestration import node_event, sensor + +ORDERS_SELECTION = "stg_orders" + +on_orders_computed = sensor( + "on_orders_computed_sensor", + description="Refresh totals when staged orders change", + when=node_event(select=ORDERS_SELECTION, kinds=["success"]), + run="orders_folder_job", +) diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_failure_sensor.py b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_failure_sensor.py new file mode 100644 index 000000000..4557faaa4 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_failure_sensor.py @@ -0,0 +1,14 @@ +"""Starts the repair job when any of four models fails: one over the limit, bound to a name.""" + +from sqlbuild.orchestration import node_event, sensor + +ORDERS_FAILED = node_event( + select="stg_orders stg_payments int_orders order_totals", kinds=["failure"] +) + +on_orders_failure = sensor( + "on_orders_failure_sensor", + description="Repair orders after a failure", + when=ORDERS_FAILED, + run="orders_repair_job", +) diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_landed_sensor.py b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_landed_sensor.py new file mode 100644 index 000000000..33c4d5687 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_orders_landed_sensor.py @@ -0,0 +1,12 @@ +"""Refreshes totals when orders land: a condition imported from a project module, over the limit.""" + +from orchestration.sensors._orders_conditions import ORDERS_LANDED + +from sqlbuild.orchestration import sensor + +on_orders_landed = sensor( + "on_orders_landed_sensor", + description="Refresh order totals after orders land", + when=ORDERS_LANDED, + run="orders_folder_job", +) diff --git a/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_tick_overdue_sensor.py b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_tick_overdue_sensor.py new file mode 100644 index 000000000..d1ddca6d6 --- /dev/null +++ b/benchmarks/duckshop/edge_cases/named_selectors/orchestration/sensors/on_tick_overdue_sensor.py @@ -0,0 +1,12 @@ +"""Repairs orders when a tick is overdue: an imported platform event, which selects no nodes.""" + +from orchestration.sensors._orders_conditions import TICK_OVERDUE + +from sqlbuild.orchestration import sensor + +on_tick_overdue = sensor( + "on_tick_overdue_sensor", + description="Repair orders after an overdue tick", + when=TICK_OVERDUE, + run="orders_repair_job", +) diff --git a/crates/sqlbuild-analysis/src/graph/main/mod.rs b/crates/sqlbuild-analysis/src/graph/main/mod.rs index 05d0272f4..c772010b6 100644 --- a/crates/sqlbuild-analysis/src/graph/main/mod.rs +++ b/crates/sqlbuild-analysis/src/graph/main/mod.rs @@ -7,6 +7,7 @@ pub mod edge_keys; pub mod graph_from_indexes; pub mod match_selector; pub mod model_layer_count; +pub mod named_selector_atoms; pub mod parse_selector; pub mod reachable_graph_keys; pub mod reachable_keys; diff --git a/crates/sqlbuild-analysis/src/graph/main/named_selector_atoms.rs b/crates/sqlbuild-analysis/src/graph/main/named_selector_atoms.rs new file mode 100644 index 000000000..2565f78b9 --- /dev/null +++ b/crates/sqlbuild-analysis/src/graph/main/named_selector_atoms.rs @@ -0,0 +1,28 @@ +use crate::graph::constants::{INTERSECTION_SEPARATOR, JOB_KIND, PATH_KIND, TAG_KIND}; +use crate::graph::models::ParsedSelector; + +/// The atoms of `selection` that name one node: bare or typed names, without `+`, globs, tags, paths or jobs. +#[must_use] +pub fn named_selector_atoms(selection: &str) -> Vec { + selection + .split(char::is_whitespace) + .flat_map(|token| token.split(INTERSECTION_SEPARATOR)) + .filter(|atom| names_one_node(atom)) + .map(str::to_owned) + .collect() +} + +fn names_one_node(atom: &str) -> bool { + match crate::graph::_helpers::selectors::parse(atom) { + Ok(ParsedSelector::Kind { + kind, + value, + upstream: false, + downstream: false, + }) => { + ![TAG_KIND, PATH_KIND, JOB_KIND].contains(&kind.as_str()) + && !crate::graph::_helpers::selectors::is_pattern(&value) + } + _ => false, + } +} diff --git a/crates/sqlbuild-analysis/src/graph/tests.rs b/crates/sqlbuild-analysis/src/graph/tests.rs index 2629f1cac..27c0469a1 100644 --- a/crates/sqlbuild-analysis/src/graph/tests.rs +++ b/crates/sqlbuild-analysis/src/graph/tests.rs @@ -4,6 +4,8 @@ mod graph; mod helpers; #[path = "tests/test_job_selectors.rs"] mod job_selectors; +#[path = "tests/test_named_selector_atoms.rs"] +mod named_selector_atoms; #[path = "tests/test_selector_scenarios.rs"] mod selector_scenarios; #[path = "tests/test_types.rs"] diff --git a/crates/sqlbuild-analysis/src/graph/tests/test_named_selector_atoms.rs b/crates/sqlbuild-analysis/src/graph/tests/test_named_selector_atoms.rs new file mode 100644 index 000000000..d176cccfd --- /dev/null +++ b/crates/sqlbuild-analysis/src/graph/tests/test_named_selector_atoms.rs @@ -0,0 +1,66 @@ +use crate::graph::main::named_selector_atoms::named_selector_atoms; +use crate::graph::tests::test_types::NamedAtomsTestCase; + +#[test] +fn given_a_selection_when_listing_named_atoms_then_only_single_node_names_count() { + let test_cases = [ + NamedAtomsTestCase { + description: "bare names count", + selection: "refund_review fct_orders", + expected_atoms: &["refund_review", "fct_orders"], + }, + NamedAtomsTestCase { + description: "a bare name inside an intersection counts, its tag does not", + selection: "fct_orders,tag:orders", + expected_atoms: &["fct_orders"], + }, + NamedAtomsTestCase { + description: "typed names count", + selection: "task:refund_review source:raw_orders seed:countries test:orders_unique", + expected_atoms: &[ + "task:refund_review", + "source:raw_orders", + "seed:countries", + "test:orders_unique", + ], + }, + NamedAtomsTestCase { + description: "tags do not count", + selection: "tag:orders tag:finance", + expected_atoms: &[], + }, + NamedAtomsTestCase { + description: "folder paths do not count", + selection: "models/orders python/orders path:models/finance", + expected_atoms: &[], + }, + NamedAtomsTestCase { + description: "graph expansions and path selectors do not count", + selection: "+fct_orders fct_orders+ +fct_refunds+ stg_orders~fct_orders", + expected_atoms: &[], + }, + NamedAtomsTestCase { + description: "globs do not count", + selection: "fct_orders_* task:refund_? fct_[ab]", + expected_atoms: &[], + }, + NamedAtomsTestCase { + description: "job selectors do not count", + selection: "job:orders_nightly_job", + expected_atoms: &[], + }, + NamedAtomsTestCase { + description: "extra whitespace adds nothing", + selection: " fct_orders \n\t fct_refunds ", + expected_atoms: &["fct_orders", "fct_refunds"], + }, + ]; + for test_case in test_cases { + assert_eq!( + named_selector_atoms(test_case.selection), + test_case.expected_atoms, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/graph/tests/test_types.rs b/crates/sqlbuild-analysis/src/graph/tests/test_types.rs index 851e74613..096f86e04 100644 --- a/crates/sqlbuild-analysis/src/graph/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/graph/tests/test_types.rs @@ -73,3 +73,9 @@ pub(super) struct NeedsJobsTestCase { pub(super) also_known: &'static [&'static str], pub(super) expected_needs_jobs: bool, } + +pub(super) struct NamedAtomsTestCase { + pub(super) description: &'static str, + pub(super) selection: &'static str, + pub(super) expected_atoms: &'static [&'static str], +} diff --git a/crates/sqlbuild-discovery/src/orchestration_files/_helpers/python_modules.rs b/crates/sqlbuild-discovery/src/orchestration_files/_helpers/python_modules.rs index f7c3b11ab..d299717f2 100644 --- a/crates/sqlbuild-discovery/src/orchestration_files/_helpers/python_modules.rs +++ b/crates/sqlbuild-discovery/src/orchestration_files/_helpers/python_modules.rs @@ -148,5 +148,6 @@ pub(crate) fn module_declaration( position: name.position, job_references, targets: declaration.targets, + condition: declaration.condition, })) } diff --git a/crates/sqlbuild-discovery/src/orchestration_files/models.rs b/crates/sqlbuild-discovery/src/orchestration_files/models.rs index 26fbd25f5..985ef4db9 100644 --- a/crates/sqlbuild-discovery/src/orchestration_files/models.rs +++ b/crates/sqlbuild-discovery/src/orchestration_files/models.rs @@ -4,6 +4,8 @@ use std::collections::BTreeMap; use serde::Deserialize; +use sqlbuild_pysource::orchestration_modules::models::ConditionSource; + use crate::models::{DiscoveryFailure, ReadFailure, StageFailure}; use crate::project_validation::models::OrchestrationNameFact; @@ -107,6 +109,8 @@ pub struct NamedModule { pub job_references: Vec<(&'static str, Positioned)>, /// A sensor's targets: empty for every target, `None` when they are not written as literals. pub targets: Option>, + /// What a sensor's `when` selects, as far as its own module shows; `None` when it selects no nodes. + pub condition: Option, } /// The calendars, sensors and every module below their folders. diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/calls.rs b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/calls.rs index d4fd1b078..e2d0c3bd9 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/calls.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/calls.rs @@ -8,12 +8,21 @@ use ruff_text_size::{Ranged, TextSize}; use crate::cpython_tree::main::source_position::source_position; use crate::cpython_tree::models::SourceLines; use crate::orchestration_modules::constants::{ - CALENDAR_CALL, ORCHESTRATION_MODULE, RUN_EVENT_CALL, SENSOR_CALL, + CALENDAR_CALL, ORCHESTRATION_MODULE, PLATFORM_EVENT_CALL, RUN_EVENT_CALL, + SELECTING_CONDITION_CALLS, SENSOR_CALL, }; use crate::orchestration_modules::models::{LiteralArgument, SourcePosition}; -/// The calls a module's `calendar`, `sensor` and `run_event` names can make. -const AUTHORING_CALLS: [&str; 3] = [CALENDAR_CALL, SENSOR_CALL, RUN_EVENT_CALL]; +/// The calls a module's `calendar`, `sensor` and condition names can make. +const AUTHORING_CALLS: [&str; 7] = [ + CALENDAR_CALL, + SENSOR_CALL, + RUN_EVENT_CALL, + PLATFORM_EVENT_CALL, + SELECTING_CONDITION_CALLS[0], + SELECTING_CONDITION_CALLS[1], + SELECTING_CONDITION_CALLS[2], +]; /// The 1-based line and column of `offset`. pub(crate) fn position(lines: &SourceLines, offset: TextSize) -> SourcePosition { @@ -96,7 +105,7 @@ fn dotted(expression: &Expr) -> Option { } } -/// Which authoring call `call` is: an imported `calendar`, `sensor` or `run_event`, or one reached as an attribute of the orchestration module. +/// Which authoring call `call` is: an imported `calendar`, `sensor` or condition, or one reached as an attribute of the orchestration module. pub(crate) fn authoring_call( call: &ExprCall, (imported, modules): (&BTreeMap<&str, &str>, &BTreeSet), diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/conditions.rs b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/conditions.rs new file mode 100644 index 000000000..7fa2aaadf --- /dev/null +++ b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/conditions.rs @@ -0,0 +1,179 @@ +//! A sensor condition's selection, following names bound at module level and noting names imported from elsewhere. + +use std::collections::BTreeMap; + +use ruff_python_ast::{Expr, ExprCall, Stmt}; +use ruff_text_size::{Ranged, TextSize}; + +use crate::orchestration_modules::_helpers::calls::{authoring_call, literal, position}; +use crate::orchestration_modules::constants::{ + EXCERPT_LIMIT, SELECT_KEYWORD, SELECTING_CONDITION_CALLS, +}; +use crate::orchestration_modules::models::{ + ConditionSource, ImportedName, LiteralArgument, ModuleScope, +}; + +/// Each module-level `NAME = value` or `NAME: T = value` with one plain name target, in source order. +pub(crate) fn module_assignments(body: &[Stmt]) -> BTreeMap<&str, Vec<(TextSize, &Expr)>> { + let mut assignments: BTreeMap<&str, Vec<(TextSize, &Expr)>> = BTreeMap::new(); + for statement in body { + if let Some((name, value)) = assignment(statement) { + assignments + .entry(name) + .or_default() + .push((statement.range().start(), value)); + } + } + assignments +} + +/// Each name's module-level `from ... import` bindings, in source order. +pub(crate) fn module_imports(body: &[Stmt]) -> BTreeMap<&str, Vec>> { + let mut imports: BTreeMap<&str, Vec>> = BTreeMap::new(); + for statement in body { + let Stmt::ImportFrom(import) = statement else { + continue; + }; + let module: String = import + .module + .as_ref() + .map(ToString::to_string) + .unwrap_or_default(); + for alias in &import.names { + let local: &str = alias.asname.as_ref().unwrap_or(&alias.name).as_str(); + imports.entry(local).or_default().push(ImportedName { + module: module.clone(), + level: import.level, + name: alias.name.as_str(), + start: statement.range().start(), + }); + } + } + imports +} + +/// What `value` selects when it runs at `before`: a selecting call's `select`, a name imported from elsewhere, or what cannot be followed. +pub(crate) fn condition_source( + value: &Expr, + before: TextSize, + scope: &ModuleScope<'_>, +) -> Option { + let mut current: (&Expr, TextSize) = (value, before); + loop { + let (expression, at) = current; + match expression { + Expr::NoneLiteral(_) => return None, + Expr::Call(call) => return call_source(call, scope), + Expr::Name(name) => { + let id: &str = name.id.as_str(); + let assigned = scope.assignments.get(id).and_then(|bindings| { + bindings + .iter() + .rev() + .find(|(start, _)| *start < at) + .copied() + }); + let imported = scope + .imports + .get(id) + .and_then(|bindings| bindings.iter().rev().find(|import| import.start < at)); + match (assigned, imported) { + (Some((start, bound)), imported) + if imported.is_none_or(|import| import.start < start) => + { + current = (bound, start); + } + (_, Some(import)) => return Some(imported_source(import, value, scope)), + _ => { + return Some(unresolvable( + format!("`{id}` is not bound at module level"), + value, + scope, + )); + } + } + } + other => { + return Some(unresolvable( + format!( + "`{}` is not a condition call or a name", + excerpt(other, scope) + ), + value, + scope, + )); + } + } + } +} + +/// A selecting call's `select`; nothing for a condition that selects no nodes; any other call cannot be followed. +fn call_source(call: &ExprCall, scope: &ModuleScope<'_>) -> Option { + match authoring_call(call, (&scope.imported, &scope.modules)) { + Some(name) if SELECTING_CONDITION_CALLS.contains(&name) => Some( + ConditionSource::Selection(match call.arguments.find_keyword(SELECT_KEYWORD) { + Some(select) => literal(&select.value, scope.lines), + None => LiteralArgument { + value: None, + position: position(scope.lines, call.range().start()), + }, + }), + ), + Some(_) => None, + None => Some(unresolvable( + format!("`{}(...)` is a function call", excerpt(&call.func, scope)), + &call.func, + scope, + )), + } +} + +/// The imported name `import` binds, reported where `at` uses it. +pub(crate) fn imported_source( + import: &ImportedName<'_>, + at: &Expr, + scope: &ModuleScope<'_>, +) -> ConditionSource { + ConditionSource::Imported { + module: import.module.clone(), + level: import.level, + name: import.name.to_owned(), + position: position(scope.lines, at.range().start()), + } +} + +fn unresolvable(what: String, at: &Expr, scope: &ModuleScope<'_>) -> ConditionSource { + ConditionSource::Unresolvable { + what, + position: position(scope.lines, at.range().start()), + } +} + +/// The first line of `expression`'s source, shortened to a quotable length. +fn excerpt(expression: &Expr, scope: &ModuleScope<'_>) -> String { + let text: &str = scope + .contents + .get(std::ops::Range::::from(expression.range())) + .unwrap_or_default() + .lines() + .next() + .unwrap_or_default(); + match text.char_indices().nth(EXCERPT_LIMIT) { + Some((cut, _)) => format!("{}...", &text[..cut]), + None => text.to_owned(), + } +} + +fn assignment(statement: &Stmt) -> Option<(&str, &Expr)> { + match statement { + Stmt::Assign(assign) => match assign.targets.as_slice() { + [Expr::Name(target)] => Some((target.id.as_str(), assign.value.as_ref())), + _ => None, + }, + Stmt::AnnAssign(assign) => match (assign.target.as_ref(), assign.value.as_deref()) { + (Expr::Name(target), Some(value)) => Some((target.id.as_str(), value)), + _ => None, + }, + _ => None, + } +} diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/declarations.rs b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/declarations.rs new file mode 100644 index 000000000..90a6ea55b --- /dev/null +++ b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/declarations.rs @@ -0,0 +1,112 @@ +//! The declaration each orchestration authoring call makes, with the selections its conditions name. + +use std::collections::BTreeMap; + +use ruff_python_ast::{Expr, ExprCall}; +use ruff_text_size::{Ranged, TextSize}; + +use crate::cpython_tree::main::walk_breadth_first::walk_breadth_first; +use crate::cpython_tree::models::{CpythonNode, ParsedSource, SourceLines}; +use crate::orchestration_modules::_helpers::calls::{ + authoring_call, literal, literal_strings, module_names, position, +}; +use crate::orchestration_modules::_helpers::conditions::{ + condition_source, module_assignments, module_imports, +}; +use crate::orchestration_modules::constants::{ + JOB_KEYWORD, NAME_KEYWORD, RUN_EVENT_CALL, RUN_KEYWORD, TARGETS_KEYWORD, WHEN_KEYWORD, +}; +use crate::orchestration_modules::models::{ + ConditionSource, LiteralArgument, ModuleScope, PythonDeclaration, PythonDeclarationKind, +}; + +/// The names `parsed`'s authoring calls and conditions resolve against, with `imports` from `imported_names`. +pub(crate) fn module_scope<'a>( + parsed: &'a ParsedSource, + contents: &'a str, + imports: &'a [(String, String, TextSize)], +) -> ModuleScope<'a> { + ModuleScope { + imported: imports + .iter() + .map(|(local, name, _)| (local.as_str(), name.as_str())) + .collect::>(), + modules: module_names(&parsed.module.body), + assignments: module_assignments(&parsed.module.body), + imports: module_imports(&parsed.module.body), + lines: &parsed.lines, + contents, + } +} + +/// The declaration one `calendar(...)` or `sensor(...)` call makes. +pub(crate) fn call_declaration<'a>( + call: &'a ExprCall, + kind: PythonDeclarationKind, + scope: &ModuleScope<'a>, +) -> PythonDeclaration { + let lines: &SourceLines = scope.lines; + let name: Option = call + .arguments + .find_argument_value(NAME_KEYWORD, 0) + .map(|value| literal(value, lines)); + let targets: Option> = match call.arguments.find_keyword(TARGETS_KEYWORD) { + Some(targets) => literal_strings(&targets.value), + None => Some(Vec::new()), + }; + let (job_references, condition) = if kind == PythonDeclarationKind::Sensor { + sensor_references(call, scope) + } else { + (Vec::new(), None) + }; + PythonDeclaration { + kind, + name, + position: position(lines, call.range().start()), + job_references, + targets, + condition, + } +} + +/// The sensor's `run` job, the jobs its run events name, and what its `when` condition selects. +fn sensor_references<'a>( + call: &'a ExprCall, + scope: &ModuleScope<'a>, +) -> ( + Vec<(&'static str, LiteralArgument)>, + Option, +) { + let lines: &SourceLines = scope.lines; + let mut job_references: Vec<(&'static str, LiteralArgument)> = Vec::new(); + if let Some(run) = call.arguments.find_keyword(RUN_KEYWORD) { + job_references.push((RUN_KEYWORD, literal(&run.value, lines))); + } + let conditions: Vec> = call + .arguments + .keywords + .iter() + .filter(|keyword| { + keyword + .arg + .as_ref() + .is_some_and(|arg| arg.as_str() != RUN_KEYWORD) + }) + .map(|keyword| CpythonNode::Expr(&keyword.value)) + .collect(); + for inner in walk_breadth_first(conditions.clone()) { + let CpythonNode::Expr(Expr::Call(inner)) = inner else { + continue; + }; + if authoring_call(inner, (&scope.imported, &scope.modules)) == Some(RUN_EVENT_CALL) + && let Some(job) = inner.arguments.find_keyword(JOB_KEYWORD) + { + job_references.push((RUN_EVENT_CALL, literal(&job.value, lines))); + } + } + let condition: Option = call + .arguments + .find_keyword(WHEN_KEYWORD) + .and_then(|when| condition_source(&when.value, call.range().start(), scope)); + (job_references, condition) +} diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/mod.rs b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/mod.rs index aedb5c489..1208b1cb5 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/mod.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/_helpers/mod.rs @@ -1 +1,3 @@ pub(crate) mod calls; +pub(crate) mod conditions; +pub(crate) mod declarations; diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/constants.rs b/crates/sqlbuild-pysource/src/orchestration_modules/constants.rs index bf2be39eb..48ea64704 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/constants.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/constants.rs @@ -6,6 +6,17 @@ pub(crate) const CALENDAR_CALL: &str = "calendar"; pub(crate) const SENSOR_CALL: &str = "sensor"; /// The sensor condition that names the job whose runs it watches. pub(crate) const RUN_EVENT_CALL: &str = "run_event"; +/// The sensor condition that watches platform events and selects no nodes. +pub(crate) const PLATFORM_EVENT_CALL: &str = "platform_event"; +/// The sensor keyword holding its one condition. +pub(crate) const WHEN_KEYWORD: &str = "when"; +/// The longest source excerpt a finding quotes. +pub(crate) const EXCERPT_LIMIT: usize = 60; +/// Sensor conditions that select the nodes or sources they watch. +pub(crate) const SELECTING_CONDITION_CALLS: [&str; 3] = + ["node_event", "source_new_data", "source_stale"]; +/// The keyword a selecting condition takes its selection in. +pub(crate) const SELECT_KEYWORD: &str = "select"; /// The keyword naming a declaration. pub(crate) const NAME_KEYWORD: &str = "name"; /// The sensor keyword naming the job it starts. diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/main/mod.rs b/crates/sqlbuild-pysource/src/orchestration_modules/main/mod.rs index 8630a3052..5abca2fbf 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/main/mod.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/main/mod.rs @@ -1 +1,2 @@ +pub mod module_binding_condition; pub mod orchestration_module_declarations; diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/main/module_binding_condition.rs b/crates/sqlbuild-pysource/src/orchestration_modules/main/module_binding_condition.rs new file mode 100644 index 000000000..420c9d947 --- /dev/null +++ b/crates/sqlbuild-pysource/src/orchestration_modules/main/module_binding_condition.rs @@ -0,0 +1,51 @@ +//! What a module-level name selects as a sensor condition, read from the module that binds it. + +use ruff_text_size::TextSize; + +use crate::cpython_tree::main::parse_cpython_module::parse_cpython_module; +use crate::cpython_tree::models::ParsedSource; +use crate::orchestration_modules::_helpers::calls::imported_names; +use crate::orchestration_modules::_helpers::conditions::condition_source; +use crate::orchestration_modules::_helpers::declarations::module_scope; +use crate::orchestration_modules::models::{ConditionSource, ModuleScope, SourcePosition}; + +/// What `name`, as `contents` leaves it bound, selects; `None` for a condition that selects no nodes. +#[must_use] +pub fn module_binding_condition( + contents: &str, + python_version: (u8, u8), + name: &str, +) -> Option { + let start = SourcePosition { line: 1, column: 1 }; + let Some(parsed): Option = parse_cpython_module(contents, python_version) else { + return Some(ConditionSource::Unresolvable { + what: format!( + "the module binding `{name}` is not valid Python {}.{}", + python_version.0, python_version.1 + ), + position: start, + }); + }; + let imports: Vec<(String, String, TextSize)> = imported_names(&parsed.module.body); + let scope: ModuleScope<'_> = module_scope(&parsed, contents, &imports); + let assigned = scope + .assignments + .get(name) + .and_then(|bindings| bindings.last()); + let imported = scope.imports.get(name).and_then(|bindings| bindings.last()); + match (assigned, imported) { + (Some((at, value)), imported) if imported.is_none_or(|import| import.start < *at) => { + condition_source(value, *at, &scope) + } + (_, Some(import)) => Some(ConditionSource::Imported { + module: import.module.clone(), + level: import.level, + name: import.name.to_owned(), + position: start, + }), + _ => Some(ConditionSource::Unresolvable { + what: format!("`{name}` is not bound at module level"), + position: start, + }), + } +} diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/main/orchestration_module_declarations.rs b/crates/sqlbuild-pysource/src/orchestration_modules/main/orchestration_module_declarations.rs index c05f52ade..69a985e4e 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/main/orchestration_module_declarations.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/main/orchestration_module_declarations.rs @@ -1,22 +1,16 @@ //! The calendars and sensors a module declares, and the job names they use, without importing it. -use std::collections::{BTreeMap, BTreeSet}; - use ruff_python_ast::Expr; -use ruff_text_size::{Ranged, TextSize}; +use ruff_text_size::TextSize; use crate::cpython_tree::main::parse_cpython_module::parse_cpython_module; use crate::cpython_tree::main::walk_breadth_first::walk_breadth_first; use crate::cpython_tree::models::{CpythonNode, ParsedSource}; -use crate::orchestration_modules::_helpers::calls::{ - authoring_call, imported_names, literal, literal_strings, module_names, position, -}; -use crate::orchestration_modules::constants::{ - CALENDAR_CALL, JOB_KEYWORD, NAME_KEYWORD, REMOVED_NAMES, RUN_EVENT_CALL, RUN_KEYWORD, - SENSOR_CALL, TARGETS_KEYWORD, -}; +use crate::orchestration_modules::_helpers::calls::{authoring_call, imported_names, position}; +use crate::orchestration_modules::_helpers::declarations::{call_declaration, module_scope}; +use crate::orchestration_modules::constants::{CALENDAR_CALL, REMOVED_NAMES, SENSOR_CALL}; use crate::orchestration_modules::models::{ - LiteralArgument, ModuleDeclarations, PythonDeclaration, PythonDeclarationKind, SourcePosition, + ModuleDeclarations, ModuleScope, PythonDeclaration, PythonDeclarationKind, SourcePosition, }; /// Every `calendar(...)` and `sensor(...)` call with its name and job references, in source order; `None` when unparsable. @@ -27,70 +21,23 @@ pub fn orchestration_module_declarations( ) -> Option { let parsed: ParsedSource = parse_cpython_module(contents, python_version)?; let imports: Vec<(String, String, TextSize)> = imported_names(&parsed.module.body); - let imported: BTreeMap<&str, &str> = imports - .iter() - .map(|(local, name, _)| (local.as_str(), name.as_str())) - .collect(); - let modules: BTreeSet = module_names(&parsed.module.body); let removed_imports: Vec<(String, SourcePosition)> = imports .iter() .filter(|(_, name, _)| REMOVED_NAMES.contains(&name.as_str())) .map(|(_, name, start)| (name.clone(), position(&parsed.lines, *start))) .collect(); + let scope: ModuleScope<'_> = module_scope(&parsed, contents, &imports); let mut declarations: Vec = Vec::new(); for node in walk_breadth_first(parsed.module.body.iter().map(CpythonNode::Stmt).collect()) { let CpythonNode::Expr(Expr::Call(call)) = node else { continue; }; - let kind = match authoring_call(call, (&imported, &modules)) { + let kind = match authoring_call(call, (&scope.imported, &scope.modules)) { Some(CALENDAR_CALL) => PythonDeclarationKind::Calendar, Some(SENSOR_CALL) => PythonDeclarationKind::Sensor, _ => continue, }; - let name: Option = call - .arguments - .find_argument_value(NAME_KEYWORD, 0) - .map(|value| literal(value, &parsed.lines)); - let mut job_references: Vec<(&'static str, LiteralArgument)> = Vec::new(); - let targets: Option> = match call.arguments.find_keyword(TARGETS_KEYWORD) { - Some(targets) => literal_strings(&targets.value), - None => Some(Vec::new()), - }; - if kind == PythonDeclarationKind::Sensor { - if let Some(run) = call.arguments.find_keyword(RUN_KEYWORD) { - job_references.push((RUN_KEYWORD, literal(&run.value, &parsed.lines))); - } - let conditions: Vec> = call - .arguments - .keywords - .iter() - .filter(|keyword| { - keyword - .arg - .as_ref() - .is_some_and(|arg| arg.as_str() != RUN_KEYWORD) - }) - .map(|keyword| CpythonNode::Expr(&keyword.value)) - .collect(); - for inner in walk_breadth_first(conditions) { - let CpythonNode::Expr(Expr::Call(inner)) = inner else { - continue; - }; - if authoring_call(inner, (&imported, &modules)) != Some(RUN_EVENT_CALL) { - continue; - } - if let Some(job) = inner.arguments.find_keyword(JOB_KEYWORD) { - job_references.push((RUN_EVENT_CALL, literal(&job.value, &parsed.lines))); - } - } - } - declarations.push(PythonDeclaration { - kind, - name, - position: position(&parsed.lines, call.range().start()), - job_references, - targets, - }); + declarations.push(call_declaration(call, kind, &scope)); } declarations .sort_by_key(|declaration| (declaration.position.line, declaration.position.column)); diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/models.rs b/crates/sqlbuild-pysource/src/orchestration_modules/models.rs index 108760147..a971f2f96 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/models.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/models.rs @@ -1,5 +1,12 @@ //! The orchestration declarations a Python module makes, read without running it. +use std::collections::{BTreeMap, BTreeSet}; + +use ruff_python_ast::Expr; +use ruff_text_size::TextSize; + +use crate::cpython_tree::models::SourceLines; + /// What a module-level `calendar(...)` or `sensor(...)` call declares. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum PythonDeclarationKind { @@ -32,6 +39,27 @@ pub struct PythonDeclaration { pub job_references: Vec<(&'static str, LiteralArgument)>, /// A sensor's `targets`: empty when omitted, `None` when not a literal tuple or list of strings. pub targets: Option>, + /// What a sensor's `when` selects, as far as its own module shows; `None` for a condition that selects no nodes. + pub condition: Option, +} + +/// Where a sensor condition's selection comes from. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ConditionSource { + /// The `select` of a `node_event`, `source_new_data` or `source_stale` call; `None` when not a string literal. + Selection(LiteralArgument), + /// A name imported from `module`, `level` packages up for a relative import, to resolve there. + Imported { + module: String, + level: u32, + name: String, + position: SourcePosition, + }, + /// A value the reader cannot follow, described for the finding. + Unresolvable { + what: String, + position: SourcePosition, + }, } /// The orchestration facts of one module. @@ -41,3 +69,25 @@ pub struct ModuleDeclarations { /// Each removed authoring name the module imports from `sqlbuild.orchestration`. pub removed_imports: Vec<(String, SourcePosition)>, } + +/// The names a module's authoring calls resolve against: its imports, imported modules and module-level bindings. +#[derive(Debug)] +pub(crate) struct ModuleScope<'a> { + pub(crate) imported: BTreeMap<&'a str, &'a str>, + pub(crate) modules: BTreeSet, + /// Each module-level name's bindings, in source order. + pub(crate) assignments: BTreeMap<&'a str, Vec<(TextSize, &'a Expr)>>, + /// Each name's module-level `from ... import` bindings, in source order. + pub(crate) imports: BTreeMap<&'a str, Vec>>, + pub(crate) lines: &'a SourceLines, + pub(crate) contents: &'a str, +} + +/// A name a module-level `from ... import` binds. +#[derive(Debug)] +pub(crate) struct ImportedName<'a> { + pub(crate) module: String, + pub(crate) level: u32, + pub(crate) name: &'a str, + pub(crate) start: TextSize, +} diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/tests/helpers.rs b/crates/sqlbuild-pysource/src/orchestration_modules/tests/helpers.rs index ac4918ab9..41993cf61 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/tests/helpers.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/tests/helpers.rs @@ -36,5 +36,6 @@ pub(super) fn declaration( position, job_references, targets: Some(Vec::new()), + condition: None, } } diff --git a/crates/sqlbuild-pysource/src/orchestration_modules/tests/test_orchestration_module_declarations.rs b/crates/sqlbuild-pysource/src/orchestration_modules/tests/test_orchestration_module_declarations.rs index ab2164368..e16bd6fc9 100644 --- a/crates/sqlbuild-pysource/src/orchestration_modules/tests/test_orchestration_module_declarations.rs +++ b/crates/sqlbuild-pysource/src/orchestration_modules/tests/test_orchestration_module_declarations.rs @@ -1,5 +1,7 @@ use crate::orchestration_modules::main::orchestration_module_declarations::orchestration_module_declarations; -use crate::orchestration_modules::models::{ModuleDeclarations, PythonDeclarationKind}; +use crate::orchestration_modules::models::{ + ConditionSource, ModuleDeclarations, PythonDeclaration, PythonDeclarationKind, +}; use crate::orchestration_modules::tests::helpers::{at, declaration, non_literal, text}; use crate::orchestration_modules::tests::test_types::ScanTestCase; @@ -35,6 +37,64 @@ fn given_orchestration_modules_when_scanning_then_reads_declarations_and_removed removed_imports: Vec::new(), }), }, + ScanTestCase { + description: "each selecting condition keeps its selection, literal or not, under any import name", + contents: "from sqlbuild.orchestration import node_event, sensor, source_new_data as fresh\nSELECTION = \"source:raw_orders\"\na = sensor(\"a_sensor\", description=\"d\", when=node_event(select=\"fct_orders fct_refunds\", kinds=[]), run=\"a_job\")\nb = sensor(\"b_sensor\", description=\"d\", when=fresh(select=SELECTION), run=\"b_job\")\n", + expected_module: Some(ModuleDeclarations { + declarations: vec![ + PythonDeclaration { + condition: Some(ConditionSource::Selection(text( + "fct_orders fct_refunds", + 3, + 64, + ))), + ..declaration( + PythonDeclarationKind::Sensor, + text("a_sensor", 3, 12), + at(3, 5), + vec![("run", text("a_job", 3, 105))], + ) + }, + PythonDeclaration { + condition: Some(ConditionSource::Selection(non_literal(4, 59))), + ..declaration( + PythonDeclarationKind::Sensor, + text("b_sensor", 4, 12), + at(4, 5), + vec![("run", text("b_job", 4, 75))], + ) + }, + ], + removed_imports: Vec::new(), + }), + }, + ScanTestCase { + description: "a condition bound to a module-level name is followed, and one whose select is unpacked cannot be counted", + contents: "from sqlbuild.orchestration import node_event, sensor\nFAILED = node_event(select=\"a b c d\", kinds=[\"failure\"])\nOPTIONS = {\"select\": \"a\"}\nc = sensor(\"c_sensor\", description=\"d\", when=FAILED, run=\"c_job\")\nd = sensor(\"d_sensor\", description=\"d\", when=node_event(**OPTIONS), run=\"d_job\")\n", + expected_module: Some(ModuleDeclarations { + declarations: vec![ + PythonDeclaration { + condition: Some(ConditionSource::Selection(text("a b c d", 2, 28))), + ..declaration( + PythonDeclarationKind::Sensor, + text("c_sensor", 4, 12), + at(4, 5), + vec![("run", text("c_job", 4, 58))], + ) + }, + PythonDeclaration { + condition: Some(ConditionSource::Selection(non_literal(5, 46))), + ..declaration( + PythonDeclarationKind::Sensor, + text("d_sensor", 5, 12), + at(5, 5), + vec![("run", text("d_job", 5, 73))], + ) + }, + ], + removed_imports: Vec::new(), + }), + }, ScanTestCase { description: "non-literal names and jobs keep their positions without values", contents: "import sqlbuild.orchestration as orchestration\n\nNAME = \"x_sensor\"\nJOB = \"x_job\"\nx = orchestration.sensor(name=NAME, description=\"d\", run=JOB)\n", @@ -103,6 +163,56 @@ fn given_orchestration_modules_when_scanning_then_reads_declarations_and_removed removed_imports: Vec::new(), }), }, + ScanTestCase { + description: "a platform event, a function call, an imported name and a rebinding after the sensor", + contents: "from orchestration.sensors._conditions import LATE as LATE_ORDERS\nfrom sqlbuild.orchestration import node_event, platform_event, sensor\nTICK = platform_event(kinds=[\"tick\"])\nC = node_event(select=\"a b\", kinds=[])\np = sensor(\"p_sensor\", description=\"d\", when=TICK, run=\"p_job\")\nf = sensor(\"f_sensor\", description=\"d\", when=make_condition(\"x\"), run=\"f_job\")\ni = sensor(\"i_sensor\", description=\"d\", when=LATE_ORDERS, run=\"i_job\")\nr = sensor(\"r_sensor\", description=\"d\", when=C, run=\"r_job\")\nC = platform_event(kinds=[])\n", + expected_module: Some(ModuleDeclarations { + declarations: vec![ + declaration( + PythonDeclarationKind::Sensor, + text("p_sensor", 5, 12), + at(5, 5), + vec![("run", text("p_job", 5, 56))], + ), + PythonDeclaration { + condition: Some(ConditionSource::Unresolvable { + what: "`make_condition(...)` is a function call".to_owned(), + position: at(6, 46), + }), + ..declaration( + PythonDeclarationKind::Sensor, + text("f_sensor", 6, 12), + at(6, 5), + vec![("run", text("f_job", 6, 71))], + ) + }, + PythonDeclaration { + condition: Some(ConditionSource::Imported { + module: "orchestration.sensors._conditions".to_owned(), + level: 0, + name: "LATE".to_owned(), + position: at(7, 46), + }), + ..declaration( + PythonDeclarationKind::Sensor, + text("i_sensor", 7, 12), + at(7, 5), + vec![("run", text("i_job", 7, 63))], + ) + }, + PythonDeclaration { + condition: Some(ConditionSource::Selection(text("a b", 4, 23))), + ..declaration( + PythonDeclarationKind::Sensor, + text("r_sensor", 8, 12), + at(8, 5), + vec![("run", text("r_job", 8, 53))], + ) + }, + ], + removed_imports: Vec::new(), + }), + }, ]; for test_case in test_cases { assert_eq!( diff --git a/crates/sqlbuild-rules/src/constants.rs b/crates/sqlbuild-rules/src/constants.rs index 4a21d14f0..2fa7cbf84 100644 --- a/crates/sqlbuild-rules/src/constants.rs +++ b/crates/sqlbuild-rules/src/constants.rs @@ -29,6 +29,8 @@ pub(crate) const NATIVE_ROWS_MEMO_VERSION: &str = "native-rules-rows-response-v2 pub(crate) const SQL_RULE_PREFIX: &str = "SQLCLEAN"; /// The Python rule family, which runs only when a selector names it or one of its codes. pub(crate) const PYTHON_RULE_PREFIX: &str = "SQBRPYTHON"; +/// The orchestration rule family, which reads job and sensor declarations rather than models. +pub(crate) const ORCHESTRATION_RULE_PREFIX: &str = "SQBRORCHESTRATION"; /// Threshold key setting the fewest test cases a custom rule must ship. pub const MIN_CUSTOM_RULE_TEST_CASES_KEY: &str = "min_custom_rule_test_cases"; /// Rules whose findings name a joined relation, so an exception may cover just that relation. diff --git a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs index e2ab69c28..51f593993 100644 --- a/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules/src/engine/_helpers/evaluation.rs @@ -1,6 +1,7 @@ use crate::configuration::main::validate as config; use crate::constants::{ - API_VERSION, NATIVE_BUILD_IDENTITY, PYTHON_RULE_PREFIX, RELATION_EXCEPTION_RULES, + API_VERSION, NATIVE_BUILD_IDENTITY, ORCHESTRATION_RULE_PREFIX, PYTHON_RULE_PREFIX, + RELATION_EXCEPTION_RULES, }; use crate::engine::_helpers::cache::{Cache, PendingBucketWrite, RuleCacheBucket, RuleCacheEntry}; use crate::lint_text::constants::LINT_UNAVAILABLE_CODE; @@ -329,9 +330,17 @@ fn evaluate_python_phase( Ok(evaluation) } -/// Built-in rules other than SQLClean and the Python rules, which read neither models nor the model cache. +/// Built-in rules other than SQLClean, the Python rules and the orchestration rules, which read neither models nor the model cache. fn reads_models(rule: &RuleMetadata) -> bool { - !rule.custom && !rule.code.starts_with("SQLCLEAN") && !rule.scope.evaluates_python() + !rule.custom + && !rule.code.starts_with("SQLCLEAN") + && !rule.scope.evaluates_python() + && !rule.code.starts_with(ORCHESTRATION_RULE_PREFIX) +} + +/// Whether a rule code or selector belongs to a family that never reaches model rules. +fn outside_model_rules(code: &str) -> bool { + code.starts_with(PYTHON_RULE_PREFIX) || code.starts_with(ORCHESTRATION_RULE_PREFIX) } fn native_rule_pool(items: usize) -> Result { @@ -361,7 +370,7 @@ fn scoped_faults( Ok(faults) } -/// Fingerprint model-rule inputs; suppressions, graph-edge exceptions and Python-rule settings never reach model rules. +/// Fingerprint model-rule inputs; suppressions, graph-edge exceptions, Python and orchestration settings never reach model rules. fn model_rules_fingerprint( selected: &[&RuleMetadata], config: &RulesConfig, @@ -372,7 +381,7 @@ fn model_rules_fingerprint( rule_options: config .rule_options .iter() - .filter(|(code, _)| !code.starts_with(PYTHON_RULE_PREFIX)) + .filter(|(code, _)| !outside_model_rules(code)) .map(|(code, options)| (code.clone(), options.clone())) .collect(), rule_exceptions: Vec::new(), @@ -384,11 +393,11 @@ fn model_rules_fingerprint( fingerprint::fingerprint(selected, &model_config, dialect) } -/// The selectors outside the Python family, which alone can name model rules. +/// The selectors outside the Python and orchestration families, which alone can name model rules. fn model_selectors(selectors: &[String]) -> Vec { selectors .iter() - .filter(|selector| !selector.starts_with(PYTHON_RULE_PREFIX)) + .filter(|selector| !outside_model_rules(selector)) .cloned() .collect() } @@ -490,7 +499,7 @@ fn evaluated_request( let model_rules: Vec<&RuleMetadata> = selected .iter() .copied() - .filter(|rule| !rule.scope.evaluates_python()) + .filter(|rule| !outside_model_rules(&rule.code)) .collect(); model_rules_fingerprint(&model_rules, &request.config, &request.dialect) }) diff --git a/crates/sqlbuild-rules/src/engine/_helpers/memo.rs b/crates/sqlbuild-rules/src/engine/_helpers/memo.rs index c119b34c6..cb8c5ca23 100644 --- a/crates/sqlbuild-rules/src/engine/_helpers/memo.rs +++ b/crates/sqlbuild-rules/src/engine/_helpers/memo.rs @@ -12,6 +12,7 @@ use crate::engine::_helpers::evaluation::{EvaluationControl, evaluate_request_wi use crate::engine::_helpers::retained::encoded_rows; use crate::errors::RowsError; use crate::models::{EvaluateRequest, Model, RetainedRows, RowsEvaluation, RowsRequest}; +use crate::orchestration_rules::main::orchestration_facts::orchestration_facts; use crate::python_rules::main::python_sources::python_sources; /// The shared key prefix of fields a request serializes but evaluation derives again. @@ -59,6 +60,7 @@ pub(crate) fn prepare(rows: RowsRequest, keyed: bool) -> Result>()?; request.python_sources = python_sources(&request).map_err(RowsError::Rules)?; + request.orchestration = orchestration_facts(&request).map_err(RowsError::Rules)?; let mut request_json: Option> = None; let memo: Option<(String, PathBuf)> = match memo { Some((code_identity, path)) => { diff --git a/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs b/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs index 17946eff4..dc5769365 100644 --- a/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs +++ b/crates/sqlbuild-rules/src/engine/tests/test_incremental_rules.rs @@ -35,6 +35,7 @@ const REVIEWED_SCOPES: &[(&str, RuleScope)] = &[ ("SQBRMODEL102", RuleScope::Model), ("SQBRMODEL103", RuleScope::Model), ("SQBRMODEL104", RuleScope::Model), + ("SQBRORCHESTRATION101", RuleScope::Project), ("SQBRPROJECT101", RuleScope::Model), ("SQBRPROJECT102", RuleScope::Model), ("SQBRPROJECT103", RuleScope::Model), diff --git a/crates/sqlbuild-rules/src/lib.rs b/crates/sqlbuild-rules/src/lib.rs index c74ed8db1..8e5e95927 100644 --- a/crates/sqlbuild-rules/src/lib.rs +++ b/crates/sqlbuild-rules/src/lib.rs @@ -12,6 +12,7 @@ pub mod errors; pub mod fix_verification; pub mod lint_text; pub mod models; +pub mod orchestration_rules; pub mod project_tree; pub mod python_rules; pub mod query_facts; diff --git a/crates/sqlbuild-rules/src/models.rs b/crates/sqlbuild-rules/src/models.rs index 4a9f740f2..e50fe328a 100644 --- a/crates/sqlbuild-rules/src/models.rs +++ b/crates/sqlbuild-rules/src/models.rs @@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize}; use sqlbuild_analysis::sql_macros::models::TypedCall; use crate::errors::RowsError; +use crate::orchestration_rules::models::OrchestrationFacts; use serde_json::Value; use std::collections::BTreeMap; @@ -772,6 +773,8 @@ pub struct EvaluateRequest { pub python_version: (u8, u8), /// The supported project Python files, filled from disk only when a Python rule is selected. pub python_sources: Vec, + /// The job and sensor selections, filled from disk only when an orchestration rule is selected. + pub orchestration: OrchestrationFacts, } /// One project Python file, keyed in the request by the digest of its bytes. @@ -822,6 +825,7 @@ impl Default for EvaluateRequest { rules_cache_path: String::new(), python_version: (3, 12), python_sources: vec![], + orchestration: OrchestrationFacts::default(), } } } diff --git a/crates/sqlbuild-rules/src/orchestration_rules/_helpers/imports.rs b/crates/sqlbuild-rules/src/orchestration_rules/_helpers/imports.rs new file mode 100644 index 000000000..14656f28f --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/_helpers/imports.rs @@ -0,0 +1,116 @@ +//! A sensor condition imported from another project module, followed to what it selects. + +use std::collections::BTreeSet; + +use sqlbuild_core::text::main::decode_text::decode_text; +use sqlbuild_discovery::tree::models::ProjectTree; +use sqlbuild_pysource::orchestration_modules::main::module_binding_condition::module_binding_condition; +use sqlbuild_pysource::orchestration_modules::models::{ + ConditionSource, LiteralArgument, SourcePosition, +}; + +use crate::orchestration_rules::constants::{INIT_MODULE, MODULE_SUFFIX}; + +/// The project files and Python version imported conditions are read with. +#[derive(Debug)] +pub(crate) struct ImportReader<'a> { + pub(crate) tree: &'a ProjectTree, + pub(crate) python_version: (u8, u8), +} + +/// What `source`, read in the module at `path`, selects once imports of project names are followed; `None` when it selects no nodes. +pub(crate) fn resolved_condition( + reader: &ImportReader<'_>, + path: &str, + source: Option, +) -> Option { + let mut followed: BTreeSet<(String, String)> = BTreeSet::new(); + let mut current: (String, Option) = (path.to_owned(), source); + loop { + let ( + from, + Some(ConditionSource::Imported { + module, + level, + name, + position, + }), + ) = current + else { + return current.1; + }; + let unresolvable = |what: String| Some(ConditionSource::Unresolvable { what, position }); + let shown: String = format!("{}{module}", ".".repeat(level as usize)); + let Some(file) = module_file(reader.tree, &from, &module, level) else { + return unresolvable(format!( + "`{name}` is imported from `{shown}`, which is not a project module" + )); + }; + if !followed.insert((file.clone(), name.clone())) { + return unresolvable(format!("`{name}` is imported in a cycle through `{file}`")); + } + let bytes: Vec = match std::fs::read(reader.tree.absolute(&file)) { + Ok(bytes) => bytes, + Err(failure) => return unresolvable(format!("`{file}` cannot be read: {failure}")), + }; + let contents: String = match decode_text(&bytes) { + Ok(contents) => contents, + Err(_undecodable) => return unresolvable(format!("`{file}` is not valid UTF-8")), + }; + let next: Option = + module_binding_condition(&contents, reader.python_version, &name); + current = match next { + Some(ConditionSource::Selection(selection)) => { + return Some(ConditionSource::Selection(LiteralArgument { + value: selection.value, + position, + })); + } + Some(ConditionSource::Unresolvable { what, .. }) => { + return unresolvable(format!("in `{file}`, {what}")); + } + Some(imported @ ConditionSource::Imported { .. }) => { + (file, Some(with_position(imported, position))) + } + None => return None, + }; + } +} + +/// `imported` reported where the sensor first named it. +fn with_position(imported: ConditionSource, at: SourcePosition) -> ConditionSource { + match imported { + ConditionSource::Imported { + module, + level, + name, + .. + } => ConditionSource::Imported { + module, + level, + name, + position: at, + }, + other => other, + } +} + +/// The project file `module` names, imported from the file at `from` with `level` leading dots. +fn module_file(tree: &ProjectTree, from: &str, module: &str, level: u32) -> Option { + let mut base: Vec<&str> = Vec::new(); + if level > 0 { + base = from.split('/').collect(); + base.pop(); + for _ in 1..level { + base.pop()?; + } + } + base.extend(module.split('.').filter(|part| !part.is_empty())); + let stem: String = base.join("/"); + [ + format!("{stem}{MODULE_SUFFIX}"), + format!("{stem}/{INIT_MODULE}"), + ] + .into_iter() + .find(|candidate| !stem.is_empty() && tree.exists(candidate)) +} diff --git a/crates/sqlbuild-rules/src/orchestration_rules/_helpers/mod.rs b/crates/sqlbuild-rules/src/orchestration_rules/_helpers/mod.rs new file mode 100644 index 000000000..a3ec7aff0 --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/_helpers/mod.rs @@ -0,0 +1,2 @@ +pub(crate) mod imports; +pub(crate) mod selections; diff --git a/crates/sqlbuild-rules/src/orchestration_rules/_helpers/selections.rs b/crates/sqlbuild-rules/src/orchestration_rules/_helpers/selections.rs new file mode 100644 index 000000000..3f684626a --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/_helpers/selections.rs @@ -0,0 +1,95 @@ +//! The selections a project's jobs and sensors make, gathered from its orchestration inventory. + +use sqlbuild_discovery::orchestration_files::models::{ + FilePosition, NamedModule, OrchestrationInventory, +}; + +use sqlbuild_pysource::orchestration_modules::models::{ConditionSource, SourcePosition}; + +use crate::orchestration_rules::_helpers::imports::{ImportReader, resolved_condition}; +use crate::orchestration_rules::constants::{JOB_OWNER, SENSOR_OWNER}; +use crate::orchestration_rules::models::{OrchestrationFacts, OrchestrationSelection}; + +/// The selections in `inventory`, and the job and job group names a selector may use. +pub(crate) fn facts( + inventory: OrchestrationInventory, + reader: &ImportReader<'_>, +) -> OrchestrationFacts { + let mut selections: Vec = inventory + .jobs + .iter() + .map(|(path, job)| { + selection( + (path, JOB_OWNER, &job.run.name.value), + Some(job.select.value.clone()), + job.select.position, + ) + }) + .collect(); + selections.extend( + inventory + .sensors + .iter() + .filter_map(|sensor| sensor_selection(sensor, reader)), + ); + let run_names: Vec = inventory + .jobs + .iter() + .map(|(_, job)| job.run.name.value.clone()) + .chain( + inventory + .groups + .iter() + .map(|(_, group)| group.run.name.value.clone()), + ) + .collect(); + OrchestrationFacts { + selections, + run_names, + } +} + +/// The selection the sensor's condition makes, following imports of project names; none when it selects no nodes. +fn sensor_selection( + sensor: &NamedModule, + reader: &ImportReader<'_>, +) -> Option { + let owner = (sensor.path.as_str(), SENSOR_OWNER, sensor.name.as_str()); + Some( + match resolved_condition(reader, &sensor.path, sensor.condition.clone())? { + ConditionSource::Selection(select) => { + selection(owner, select.value, file_position(select.position)) + } + ConditionSource::Unresolvable { what, position } => OrchestrationSelection { + unresolved: Some(what), + ..selection(owner, None, file_position(position)) + }, + ConditionSource::Imported { position, .. } => { + selection(owner, None, file_position(position)) + } + }, + ) +} + +fn file_position(position: SourcePosition) -> FilePosition { + FilePosition { + line: position.line, + column: position.column, + } +} + +fn selection( + (path, owner, name): (&str, &str, &str), + text: Option, + position: FilePosition, +) -> OrchestrationSelection { + OrchestrationSelection { + path: path.to_owned(), + owner: owner.to_owned(), + name: name.to_owned(), + selection: text, + unresolved: None, + line: position.line as u64, + column: position.column as u64, + } +} diff --git a/crates/sqlbuild-rules/src/orchestration_rules/constants.rs b/crates/sqlbuild-rules/src/orchestration_rules/constants.rs new file mode 100644 index 000000000..d357e4d1b --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/constants.rs @@ -0,0 +1,16 @@ +//! The orchestration rule codes and the option they read. + +/// Limits how many nodes a job or sensor-condition selection names one by one. +pub(crate) const NAMED_SELECTOR_LIMIT_CODE: &str = "SQBRORCHESTRATION101"; +/// The option holding that limit. +pub(crate) const MAX_NAMED_SELECTORS_OPTION: &str = "max_named_selectors"; +/// The limit when the option is not set. +pub(crate) const DEFAULT_MAX_NAMED_SELECTORS: u32 = 3; +/// The owner label of a job selection. +pub(crate) const JOB_OWNER: &str = "job"; +/// The owner label of a sensor-condition selection. +pub(crate) const SENSOR_OWNER: &str = "sensor"; +/// The suffix of a Python module file. +pub(crate) const MODULE_SUFFIX: &str = ".py"; +/// The file a Python package is imported from. +pub(crate) const INIT_MODULE: &str = "__init__.py"; diff --git a/crates/sqlbuild-rules/src/orchestration_rules/main/evaluate_orchestration_rules.rs b/crates/sqlbuild-rules/src/orchestration_rules/main/evaluate_orchestration_rules.rs new file mode 100644 index 000000000..6b7122f6d --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/main/evaluate_orchestration_rules.rs @@ -0,0 +1,94 @@ +//! Findings of the selected orchestration rules over the request's orchestration facts. + +use std::collections::BTreeSet; + +use sqlbuild_analysis::graph::main::named_selector_atoms::named_selector_atoms; + +use crate::models::{EvaluateRequest, Fault, RuleMetadata}; +use crate::orchestration_rules::constants::{ + DEFAULT_MAX_NAMED_SELECTORS, MAX_NAMED_SELECTORS_OPTION, NAMED_SELECTOR_LIMIT_CODE, +}; +use crate::orchestration_rules::models::OrchestrationSelection; + +/// A finding for each selection that names more nodes than its limit, or cannot be counted. +pub(crate) fn evaluate_orchestration_rules( + request: &EvaluateRequest, + rule: &RuleMetadata, +) -> Vec { + let limit: u32 = request + .config + .rule_option_u32(NAMED_SELECTOR_LIMIT_CODE, MAX_NAMED_SELECTORS_OPTION) + .unwrap_or(DEFAULT_MAX_NAMED_SELECTORS); + let run_names: BTreeSet<&str> = request + .orchestration + .run_names + .iter() + .map(String::as_str) + .collect(); + request + .orchestration + .selections + .iter() + .filter_map(|selection| selection_fault(rule, selection, (limit, &run_names))) + .collect() +} + +fn selection_fault( + rule: &RuleMetadata, + selection: &OrchestrationSelection, + (limit, run_names): (u32, &BTreeSet<&str>), +) -> Option { + let OrchestrationSelection { owner, name, .. } = selection; + if let Some(what) = &selection.unresolved { + return Some(fault( + rule, + selection, + format!( + "{owner} '{name}' has a condition the rule cannot resolve: {what}, so the nodes it selects cannot be counted" + ), + "Add an exact rule exception for this file with a reason.".to_owned(), + )); + } + let Some(text) = &selection.selection else { + return Some(fault( + rule, + selection, + format!( + "{owner} '{name}' has a condition select that is not a string literal, so the nodes it names cannot be counted" + ), + "Write the select as a quoted string. If it must be computed, add an exact rule exception for this file with a reason.".to_owned(), + )); + }; + let named: BTreeSet = named_selector_atoms(text) + .into_iter() + .filter(|atom| !run_names.contains(atom.as_str())) + .collect(); + let count: usize = named.len(); + (count > limit as usize).then(|| { + fault( + rule, + selection, + format!("{owner} '{name}' names {count} nodes in its selection (limit {limit})"), + format!( + "If these nodes are one pipeline, select its folder or a tag. If the {owner} deliberately targets exactly these nodes, add an exact rule exception for this file with a reason." + ), + ) + }) +} + +fn fault( + rule: &RuleMetadata, + selection: &OrchestrationSelection, + message: String, + remediation: String, +) -> Fault { + Fault { + unevaluated: false, + code: rule.code.clone(), + path: selection.path.clone(), + line: selection.line, + column: selection.column, + message, + remediation, + } +} diff --git a/crates/sqlbuild-rules/src/orchestration_rules/main/mod.rs b/crates/sqlbuild-rules/src/orchestration_rules/main/mod.rs new file mode 100644 index 000000000..2d2b0dd9a --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/main/mod.rs @@ -0,0 +1,2 @@ +pub mod evaluate_orchestration_rules; +pub mod orchestration_facts; diff --git a/crates/sqlbuild-rules/src/orchestration_rules/main/orchestration_facts.rs b/crates/sqlbuild-rules/src/orchestration_rules/main/orchestration_facts.rs new file mode 100644 index 000000000..82ae011b6 --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/main/orchestration_facts.rs @@ -0,0 +1,31 @@ +//! The project's orchestration selections, read from disk only when an orchestration rule is selected. + +use std::path::Path; + +use sqlbuild_discovery::orchestration_files::main::orchestration_inventory::orchestration_inventory; +use sqlbuild_discovery::orchestration_files::models::OrchestrationInventory; +use sqlbuild_discovery::tree::models::ProjectTree; + +use crate::models::EvaluateRequest; +use crate::orchestration_rules::_helpers::imports::ImportReader; +use crate::orchestration_rules::_helpers::selections::facts; +use crate::orchestration_rules::constants::NAMED_SELECTOR_LIMIT_CODE; +use crate::orchestration_rules::models::OrchestrationFacts; +use crate::rules::main::request_selects::request_selects; + +/// Every job and sensor-condition selection when the request selects an orchestration rule, otherwise none. +pub(crate) fn orchestration_facts(request: &EvaluateRequest) -> Result { + if !request_selects(request, |rule| rule.code == NAMED_SELECTOR_LIMIT_CODE)? { + return Ok(OrchestrationFacts::default()); + } + let tree = ProjectTree::new(Path::new(&request.project_dir)); + let inventory: OrchestrationInventory = orchestration_inventory(&tree, request.python_version) + .map_err(|failure| format!("cannot read the orchestration declarations: {failure:?}"))?; + Ok(facts( + inventory, + &ImportReader { + tree: &tree, + python_version: request.python_version, + }, + )) +} diff --git a/crates/sqlbuild-rules/src/orchestration_rules/mod.rs b/crates/sqlbuild-rules/src/orchestration_rules/mod.rs new file mode 100644 index 000000000..e6b702c49 --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/mod.rs @@ -0,0 +1,9 @@ +//! Rules over a project's orchestration declarations: jobs and sensors. + +pub(crate) mod _helpers; +pub(crate) mod constants; +pub mod main; +pub mod models; + +#[cfg(test)] +mod tests; diff --git a/crates/sqlbuild-rules/src/orchestration_rules/models.rs b/crates/sqlbuild-rules/src/orchestration_rules/models.rs new file mode 100644 index 000000000..81d5c7549 --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/models.rs @@ -0,0 +1,27 @@ +//! The orchestration facts a rules request carries. + +use serde::{Deserialize, Serialize}; + +/// One job `select` or sensor-condition `select`, where it is written and who owns it. +#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct OrchestrationSelection { + pub path: String, + pub owner: String, + pub name: String, + /// The selection text; `None` for a sensor condition whose `select` is not a string literal. + pub selection: Option, + /// What the rule could not resolve to reach the selection, when it could not. + #[serde(default)] + pub unresolved: Option, + pub line: u64, + pub column: u64, +} + +/// The selections a project's jobs and sensors make, and the job and job group names a selector may use. +#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(default, deny_unknown_fields)] +pub struct OrchestrationFacts { + pub selections: Vec, + pub run_names: Vec, +} diff --git a/crates/sqlbuild-rules/src/orchestration_rules/tests.rs b/crates/sqlbuild-rules/src/orchestration_rules/tests.rs new file mode 100644 index 000000000..fc71d612c --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/tests.rs @@ -0,0 +1,6 @@ +#[path = "tests/helpers.rs"] +mod helpers; +#[path = "tests/test_named_selector_limit.rs"] +mod named_selector_limit; +#[path = "tests/test_types.rs"] +mod test_types; diff --git a/crates/sqlbuild-rules/src/orchestration_rules/tests/helpers.rs b/crates/sqlbuild-rules/src/orchestration_rules/tests/helpers.rs new file mode 100644 index 000000000..9fef1a1ee --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/tests/helpers.rs @@ -0,0 +1,67 @@ +use serde_json::json; + +use crate::engine::_helpers::memo::evaluate_rows; +use crate::models::{EvaluateRequest, RowsRequest}; + +/// A job declaration selecting `select`. +pub(super) fn job(name: &str, select: &str) -> String { + format!("name = \"{name}\"\ndescription = \"d\"\nselect = \"{select}\"\n") +} + +/// A sensor module whose `when` is `condition`. +pub(super) fn sensor(name: &str, condition: &str) -> String { + format!( + "from sqlbuild.orchestration import node_event, sensor, source_new_data\n\nSELECTION = \"a\"\n{name} = sensor(name=\"{name}_sensor\", description=\"d\", when={condition}, run=\"orders_export_job\")\n" + ) +} + +/// The job the sensors run. +pub(super) fn export_job() -> (&'static str, String) { + ( + "orchestration/jobs/orders/export.toml", + job("orders_export_job", "orders"), + ) +} + +/// The named-selector findings for a project of `files`, as path, line, column and message. +pub(super) fn limit_findings( + files: &[(&str, String)], + config: serde_json::Value, +) -> Result, String> { + let directory = tempfile::tempdir().map_err(|error| error.to_string())?; + for (path, contents) in files { + let file = directory.path().join(path); + std::fs::create_dir_all(file.parent().expect("parent")) + .map_err(|error| error.to_string())?; + std::fs::write(&file, contents).map_err(|error| error.to_string())?; + } + let mut config = config; + config["cache"] = json!({ "enabled": false }); + let request = EvaluateRequest { + project_dir: directory.path().to_string_lossy().into_owned(), + config: serde_json::from_value(config).map_err(|error| error.to_string())?, + ..EvaluateRequest::default() + }; + let evaluation = evaluate_rows(RowsRequest { + request, + memo: None, + retain: false, + }) + .map_err(|error| format!("{error:?}"))?; + Ok(evaluation + .faults + .into_iter() + .filter(|fault| fault.code == "SQBRORCHESTRATION101") + .map(|fault| (fault.path, fault.line, fault.column, fault.message)) + .collect()) +} + +/// Expected findings as owned path, line, column and message tuples. +pub(super) fn owned_findings(items: &[(&str, u64, u64, &str)]) -> Vec<(String, u64, u64, String)> { + items + .iter() + .map(|(path, line, column, message)| { + ((*path).to_owned(), *line, *column, (*message).to_owned()) + }) + .collect() +} diff --git a/crates/sqlbuild-rules/src/orchestration_rules/tests/test_named_selector_limit.rs b/crates/sqlbuild-rules/src/orchestration_rules/tests/test_named_selector_limit.rs new file mode 100644 index 000000000..587c84bdf --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/tests/test_named_selector_limit.rs @@ -0,0 +1,257 @@ +use serde_json::json; + +use crate::orchestration_rules::tests::helpers::{ + export_job, job, limit_findings, owned_findings, sensor, +}; +use crate::orchestration_rules::tests::test_types::NamedSelectorLimitTestCase; + +#[test] +fn given_job_and_sensor_selections_when_evaluating_then_more_named_nodes_than_the_limit_are_findings() + { + let selected = json!({ "select": ["SQBRORCHESTRATION101"] }); + let test_cases = [ + NamedSelectorLimitTestCase { + description: "four bare names exceed the default limit of three", + files: vec![ + export_job(), + ( + "orchestration/jobs/orders/nightly.toml", + job( + "orders_nightly_job", + "raw_storefront_orders partner_orders_file request_carrier_collection refund_review", + ), + ), + ], + config: selected.clone(), + expected_findings: vec![( + "orchestration/jobs/orders/nightly.toml", + 3, + 10, + "job 'orders_nightly_job' names 4 nodes in its selection (limit 3)", + )], + }, + NamedSelectorLimitTestCase { + description: "three names, an intersection's name and typed names stay within the limit", + files: vec![ + export_job(), + ( + "orchestration/jobs/orders/nightly.toml", + job( + "orders_nightly_job", + "fct_orders,tag:orders task:refund_review source:raw_orders tag:finance", + ), + ), + ], + config: selected.clone(), + expected_findings: vec![], + }, + NamedSelectorLimitTestCase { + description: "tags, folders, expansions, globs, jobs and the same name twice add nothing", + files: vec![ + export_job(), + ( + "orchestration/jobs/orders/nightly.toml", + job( + "orders_nightly_job", + "a b c a tag:orders models/orders python/orders +fct_orders fct_refunds+ fct_* job:orders_export_job orders_export_job", + ), + ), + ], + config: selected.clone(), + expected_findings: vec![], + }, + NamedSelectorLimitTestCase { + description: "the option lowers the limit", + files: vec![ + export_job(), + ( + "orchestration/jobs/orders/nightly.toml", + job("orders_nightly_job", "fct_orders fct_refunds"), + ), + ], + config: json!({ + "select": ["SQBRORCHESTRATION101"], + "rule_options": { "SQBRORCHESTRATION101": { "max_named_selectors": 1 } }, + }), + expected_findings: vec![( + "orchestration/jobs/orders/nightly.toml", + 3, + 10, + "job 'orders_nightly_job' names 2 nodes in its selection (limit 1)", + )], + }, + NamedSelectorLimitTestCase { + description: "a sensor condition is counted, and one that is not a literal is reported", + files: vec![ + export_job(), + ( + "orchestration/sensors/orders/watch.py", + sensor("orders_watch", "node_event(select=\"a b c d\", kinds=[])"), + ), + ( + "orchestration/sensors/orders/fresh.py", + sensor("orders_fresh", "source_new_data(select=SELECTION)"), + ), + ], + config: selected.clone(), + expected_findings: vec![ + ( + "orchestration/sensors/orders/fresh.py", + 4, + 96, + "sensor 'orders_fresh_sensor' has a condition select that is not a string literal, so the nodes it names cannot be counted", + ), + ( + "orchestration/sensors/orders/watch.py", + 4, + 91, + "sensor 'orders_watch_sensor' names 4 nodes in its selection (limit 3)", + ), + ], + }, + NamedSelectorLimitTestCase { + description: "imports of project names are followed, platform events are skipped, and what cannot be resolved is reported", + files: vec![ + export_job(), + ( + "orchestration/sensors/orders/_conditions.py", + "from sqlbuild.orchestration import node_event, platform_event\nLATE = node_event(select=\"a b c d\", kinds=[])\nTICK = platform_event(kinds=[])\n".to_owned(), + ), + ( + "orchestration/sensors/orders/late.py", + format!( + "from orchestration.sensors.orders._conditions import LATE\n{}", + sensor("orders_late", "LATE") + ), + ), + ( + "orchestration/sensors/orders/tick.py", + format!("from ._conditions import TICK\n{}", sensor("orders_tick", "TICK")), + ), + ( + "orchestration/sensors/orders/built.py", + sensor("orders_built", "make_condition(\"orders\")"), + ), + ( + "orchestration/sensors/orders/shared.py", + format!( + "from shared_conditions import READY\n{}", + sensor("orders_shared", "READY") + ), + ), + ], + config: selected.clone(), + expected_findings: vec![ + ( + "orchestration/sensors/orders/built.py", + 4, + 73, + "sensor 'orders_built_sensor' has a condition the rule cannot resolve: `make_condition(...)` is a function call, so the nodes it selects cannot be counted", + ), + ( + "orchestration/sensors/orders/late.py", + 5, + 71, + "sensor 'orders_late_sensor' names 4 nodes in its selection (limit 3)", + ), + ( + "orchestration/sensors/orders/shared.py", + 5, + 75, + "sensor 'orders_shared_sensor' has a condition the rule cannot resolve: `READY` is imported from `shared_conditions`, which is not a project module, so the nodes it selects cannot be counted", + ), + ], + }, + NamedSelectorLimitTestCase { + description: "a CRLF condition module is read like any other, and a name follows its last binding before the use", + files: vec![ + export_job(), + ( + "orchestration/sensors/orders/_crlf.py", + "# r\u{e9}sum\u{e9}\r\nfrom sqlbuild.orchestration import node_event\r\nX = m\u{e9}k(\"x\")\r\n".to_owned(), + ), + ( + "orchestration/sensors/orders/_wide.py", + "from sqlbuild.orchestration import node_event\nWIDE = node_event(select=\"a b c d\", kinds=[])\n".to_owned(), + ), + ( + "orchestration/sensors/orders/_rebound.py", + "from sqlbuild.orchestration import node_event\nWIDE = node_event(select=\"a\", kinds=[])\nfrom ._wide import WIDE\n".to_owned(), + ), + ( + "orchestration/sensors/orders/crlf.py", + format!("from ._crlf import X\n{}", sensor("orders_crlf", "X")), + ), + ( + "orchestration/sensors/orders/rebound.py", + format!("from ._rebound import WIDE\n{}", sensor("orders_rebound", "WIDE")), + ), + ( + "orchestration/sensors/orders/reimported.py", + format!( + "from ._wide import WIDE\n{}from ._crlf import X as WIDE\n", + sensor("orders_reimported", "WIDE") + ), + ), + ], + config: selected.clone(), + expected_findings: vec![ + ( + "orchestration/sensors/orders/crlf.py", + 5, + 71, + "sensor 'orders_crlf_sensor' has a condition the rule cannot resolve: in `orchestration/sensors/orders/_crlf.py`, `m\u{e9}k(...)` is a function call, so the nodes it selects cannot be counted", + ), + ( + "orchestration/sensors/orders/rebound.py", + 5, + 77, + "sensor 'orders_rebound_sensor' names 4 nodes in its selection (limit 3)", + ), + ( + "orchestration/sensors/orders/reimported.py", + 5, + 83, + "sensor 'orders_reimported_sensor' names 4 nodes in its selection (limit 3)", + ), + ], + }, + NamedSelectorLimitTestCase { + description: "an exact rule exception for the file removes its finding", + files: vec![ + export_job(), + ( + "orchestration/jobs/orders/nightly.toml", + job("orders_nightly_job", "a b c d"), + ), + ], + config: json!({ + "select": ["SQBRORCHESTRATION101"], + "rule_exceptions": [{ + "rule": "SQBRORCHESTRATION101", + "path": "orchestration/jobs/orders/nightly.toml", + "reason": "A repair job for four specific models.", + }], + }), + expected_findings: vec![], + }, + NamedSelectorLimitTestCase { + description: "without the rule selected, the declarations are not read", + files: vec![( + "orchestration/jobs/orders/nightly.toml", + "not = [valid".to_owned(), + )], + config: json!({ "select": ["SQBRMODEL101"] }), + expected_findings: vec![], + }, + ]; + for test_case in test_cases { + let findings = limit_findings(&test_case.files, test_case.config.clone()); + assert_eq!( + findings, + Ok(owned_findings(&test_case.expected_findings)), + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-rules/src/orchestration_rules/tests/test_types.rs b/crates/sqlbuild-rules/src/orchestration_rules/tests/test_types.rs new file mode 100644 index 000000000..b11513e59 --- /dev/null +++ b/crates/sqlbuild-rules/src/orchestration_rules/tests/test_types.rs @@ -0,0 +1,6 @@ +pub(super) struct NamedSelectorLimitTestCase { + pub(super) description: &'static str, + pub(super) files: Vec<(&'static str, String)>, + pub(super) config: serde_json::Value, + pub(super) expected_findings: Vec<(&'static str, u64, u64, &'static str)>, +} diff --git a/crates/sqlbuild-rules/src/python_rules/main/python_sources.rs b/crates/sqlbuild-rules/src/python_rules/main/python_sources.rs index f31ebf899..1ec2fdbaa 100644 --- a/crates/sqlbuild-rules/src/python_rules/main/python_sources.rs +++ b/crates/sqlbuild-rules/src/python_rules/main/python_sources.rs @@ -5,18 +5,11 @@ use std::path::Path; use sqlbuild_discovery::python_paths::main::supported_python_paths::supported_python_paths; use crate::models::{EvaluateRequest, PythonSource}; -use crate::rules::main::assemble_catalogue::assemble_catalogue; -use crate::rules::main::select::select; +use crate::rules::main::request_selects::request_selects; /// Every supported project Python file when the request selects a Python rule, otherwise none. pub(crate) fn python_sources(request: &EvaluateRequest) -> Result, String> { - let catalogue = assemble_catalogue(&request.custom_rules)?; - let selected = select( - &catalogue, - request.config.select.as_deref(), - &request.config.ignore, - )?; - if !selected.iter().any(|rule| rule.scope.evaluates_python()) { + if !request_selects(request, |rule| rule.scope.evaluates_python())? { return Ok(Vec::new()); } let project_dir: &Path = Path::new(&request.project_dir); diff --git a/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs b/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs index 283a00ce7..812a79497 100644 --- a/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs +++ b/crates/sqlbuild-rules/src/rules/_helpers/catalogue.rs @@ -43,6 +43,7 @@ macro_rules! rule { | "SQBRTEST104" | "SQBRTEST203" | "SQBRTEST301" + | "SQBRORCHESTRATION101" ), custom: false, scope: $scope, @@ -116,6 +117,14 @@ pub(crate) fn catalogue() -> Vec { "each domain's published models must share one name shape", "Name every published model of a domain either ____ or ______. Move consumer-specific variants into intermediate or the consuming domain, rename models into the family, or allowlist an intentional qualifier with a reason.", ), + rule!( + "SQBRORCHESTRATION101", + RuleScope::Project, + "orchestration", + "named-selector-limit", + "job and sensor selections must not list nodes one by one", + "Select a pipeline by its folder or a tag, which keeps nodes added later; add an exact rule exception with a reason for a job that deliberately targets specific nodes.", + ), rule!( "SQBRPROJECT101", RuleScope::Model, diff --git a/crates/sqlbuild-rules/src/rules/main/builtin_options.rs b/crates/sqlbuild-rules/src/rules/main/builtin_options.rs index 051606f1f..55bbd8805 100644 --- a/crates/sqlbuild-rules/src/rules/main/builtin_options.rs +++ b/crates/sqlbuild-rules/src/rules/main/builtin_options.rs @@ -67,6 +67,14 @@ pub fn builtin_options(code: &str) -> Vec { whose file path matches a glob.", }, ], + "SQBRORCHESTRATION101" => vec![BuiltinOption { + kind: BuiltinOptionKind::Integer { + default: 3, + minimum: 0, + }, + name: "max_named_selectors", + description: "Most nodes one job or sensor-condition selection may name one by one.", + }], "SQBRTEST203" => vec![BuiltinOption { kind: BuiltinOptionKind::StringList(&[]), name: "allowed_tests", diff --git a/crates/sqlbuild-rules/src/rules/main/evaluate_project.rs b/crates/sqlbuild-rules/src/rules/main/evaluate_project.rs index 592ef876e..0154a81db 100644 --- a/crates/sqlbuild-rules/src/rules/main/evaluate_project.rs +++ b/crates/sqlbuild-rules/src/rules/main/evaluate_project.rs @@ -1,4 +1,6 @@ use crate::models::Fault; +use crate::orchestration_rules::constants::NAMED_SELECTOR_LIMIT_CODE; +use crate::orchestration_rules::main::evaluate_orchestration_rules::evaluate_orchestration_rules; use crate::rules::models::ProjectEvaluationRequest; pub(crate) fn evaluate_project( @@ -8,6 +10,9 @@ pub(crate) fn evaluate_project( faults.extend(crate::rules::_helpers::evaluation::evaluate_project_rules( &request, )); + if let Some(rule) = request.selected.get(NAMED_SELECTOR_LIMIT_CODE) { + faults.extend(evaluate_orchestration_rules(request.request, rule)); + } faults.extend(crate::rules::_helpers::sql_test_rules::evaluate_project( request, )?); diff --git a/crates/sqlbuild-rules/src/rules/main/mod.rs b/crates/sqlbuild-rules/src/rules/main/mod.rs index b90109699..a6715a1ee 100644 --- a/crates/sqlbuild-rules/src/rules/main/mod.rs +++ b/crates/sqlbuild-rules/src/rules/main/mod.rs @@ -5,6 +5,7 @@ pub mod compact_sql; pub mod evaluate; pub mod evaluate_project; pub mod fingerprint; +pub mod request_selects; pub mod resolve_threshold_overrides; pub mod select; pub mod selected_codes; diff --git a/crates/sqlbuild-rules/src/rules/main/request_selects.rs b/crates/sqlbuild-rules/src/rules/main/request_selects.rs new file mode 100644 index 000000000..50bdadaad --- /dev/null +++ b/crates/sqlbuild-rules/src/rules/main/request_selects.rs @@ -0,0 +1,17 @@ +use crate::models::{EvaluateRequest, RuleMetadata}; +use crate::rules::main::assemble_catalogue::assemble_catalogue; +use crate::rules::main::select::select; + +/// Whether the request's configuration selects a rule that `matches`. +pub(crate) fn request_selects( + request: &EvaluateRequest, + matches: impl Fn(&RuleMetadata) -> bool, +) -> Result { + let catalogue = assemble_catalogue(&request.custom_rules)?; + let selected = select( + &catalogue, + request.config.select.as_deref(), + &request.config.ignore, + )?; + Ok(selected.into_iter().any(matches)) +} diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md index f0359245f..1f393a71a 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/CONTENTS.md @@ -49,6 +49,7 @@ Bundled copies of every page on the SQLBuild documentation site, matching the in - [Configuration and selection](concepts/rules/configuration-and-selection.md) (`concepts/rules/configuration-and-selection`) - Select built-in and custom Rules by exact code or family prefix. - [Findings and exceptions](concepts/rules/findings-and-exceptions.md) (`concepts/rules/findings-and-exceptions`) - Understand Rule diagnostics and record intentional exceptions safely. - [Python Rules](concepts/rules/python-rules.md) (`concepts/rules/python-rules`) - Check the project's Python for annotations, imports, function shape and the layout of python/. +- [Orchestration Rules](concepts/rules/orchestration-rules.md) (`concepts/rules/orchestration-rules`) - Keep job and sensor selections from listing nodes one by one. - [Execution and caching](concepts/rules/execution-and-caching.md) (`concepts/rules/execution-and-caching`) - Understand Rule enforcement order, focused runs, and dependency-aware reuse. ## Ship diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/orchestration.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/orchestration.md index d37f7e9a8..da2a6a9d3 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/orchestration.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/orchestration.md @@ -94,6 +94,10 @@ cpus = 2 A job's `[settings]` cannot set `select` or `exclude`: the job's own selection is the run's selection. +Select a pipeline by its folder or a tag rather than listing its nodes. Rule `SQBRORCHESTRATION101` +reports a job or sensor `select` that names more than three nodes one by one; see +[Orchestration Rules](rules/orchestration-rules.md). + ## Group jobs A job group runs its jobs as one run. A job waits for another job in the group when it reads a node diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md index 73274c201..89a8c77fc 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules.md @@ -163,7 +163,9 @@ SQLBuild maintains native built-in Rules for reusable requirements. SQL Rule cod `SQLCLEAN`, for example `SQLCLEAN004`. Rules that read the project, such as `SQBRGRAPH101`, begin with `SQBR`. The `SQBRPYTHON` Rules check the project's Python: annotations, imports, function shape and the layout of `python/`. They run only when `select` names `SQBRPYTHON` or one of its -codes; see [Python Rules](rules/python-rules.md). +codes; see [Python Rules](rules/python-rules.md). `SQBRORCHESTRATION101` keeps job and +sensor selections from listing nodes one by one; see +[Orchestration Rules](rules/orchestration-rules.md). Projects can define custom Python Rules under `rules/**/*.py`. Their codes begin with `XSQBR`, for example `XSQBRARCH001`. Custom Rules use the same selection, findings, exceptions, execution order, diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md index d9a093ace..79ad15afc 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/configuration-and-selection.md @@ -91,6 +91,9 @@ optional `domain`; `qualifier` and `paths` accept globs. See The Python Rules take whole-number limits such as `max_arguments` for `SQBRPYTHON504`; see [Python Rules](python-rules.md#rule-options). +`SQBRORCHESTRATION101` accepts `max_named_selectors`, the most nodes one job or sensor selection may +name one by one (default 3); see [Orchestration Rules](orchestration-rules.md). + Unknown Rule codes, option names, and invalid option values fail configuration. ## Focused selection diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/orchestration-rules.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/orchestration-rules.md new file mode 100644 index 000000000..faf94c02b --- /dev/null +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/concepts/rules/orchestration-rules.md @@ -0,0 +1,95 @@ + + +# Orchestration Rules + +> Keep job and sensor selections from listing nodes one by one. + +Online: https://sqlbuild.com/docs/concepts/rules/orchestration-rules/ + +`SQBRORCHESTRATION101` reports a job or sensor whose `select` names more than three nodes one by one. +Select a pipeline by its folder or a tag instead, so nodes added to it later are built too. + +```toml +# orchestration/jobs/nightly_orders_job.toml +name = "nightly_orders_job" +description = "Orders and their daily totals" +select = "stg_orders stg_payments int_orders order_totals" +``` + +```text +error[SQBRORCHESTRATION101]: job 'nightly_orders_job' names 4 nodes in its selection (limit 3) + --> orchestration/jobs/nightly_orders_job.toml:3:10 + = help: If these nodes are one pipeline, select its folder or a tag. If the job deliberately targets exactly these nodes, add an exact rule exception for this file with a reason. +``` + +```toml +select = "models/orders" +``` + +The Rule is selected with `SQBR` or `SQBRORCHESTRATION`, and with its exact code. + +## What counts as a named node + +The Rule reads the `select` of every [job](../orchestration.md#declare-a-job) and the +`select` of each `node_event`, `source_new_data` and `source_stale` condition of a +[sensor](../orchestration.md#start-a-job-from-a-sensor). It counts the distinct +[selector](../selectors.md) atoms that each name one node. + +| Atom | Counts | +| --- | --- | +| A bare name, such as `order_totals` | Yes | +| A typed name, such as `source:raw_orders` or `task:refund_review` | Yes | +| Each name in an intersection, such as `order_totals,tag:finance` | The name only | +| `tag:`, a folder path, `job:`, a job or job group name | No | +| A name with `+` before or after it | No | +| A glob, such as `fct_*` | No | + +A name written twice counts once. A job's `exclude` is not checked. + +## Change the limit + +Set `max_named_selectors` to the most nodes one selection may name. The default is 3, and 0 rejects +every named node. + +```toml +[rules.rule_options.SQBRORCHESTRATION101] +max_named_selectors = 5 +``` + +## Keep a deliberate list + +A repair job that targets exactly a few nodes is a deliberate exception. Record it for the job's +file with a reason: + +```toml +[[rules.rule_exceptions]] +rule = "SQBRORCHESTRATION101" +path = "orchestration/jobs/repair_orders_job.toml" +reason = "Rebuilds the four order models after a late payments load." +``` + +See [Findings and exceptions](findings-and-exceptions.md#exact-exceptions). + +## Sensor conditions the Rule cannot count + +The Rule reads a sensor's `when` without running the module. It follows a name bound at module +level, in the sensor's file or in a project module the sensor imports it from, to the condition +call that name holds. + +| `when` value | Result | +| --- | --- | +| A selecting condition whose `select` is a quoted string | Counted | +| A `platform_event` or `run_event` condition, written inline or held in a name | Not checked | +| A selecting condition whose `select` is a variable or an expression | Reported | +| A call to any other function, such as a helper that builds the condition | Reported | +| A name imported from outside the project, or not bound at module level | Reported | +| Any other expression, such as `module.NAME` | Reported | + +A reported condition names what the Rule could not resolve: + +```text +error[SQBRORCHESTRATION101]: sensor 'on_orders_built_sensor' has a condition the rule cannot resolve: `landed_after(...)` is a function call, so the nodes it selects cannot be counted +``` + +To keep a condition the Rule cannot count, add an exact rule exception for the sensor's file with a +reason. diff --git a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py index 4c22cbaf5..a847abb71 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/_test_types.py @@ -1163,6 +1163,17 @@ class SqlRuleCacheSequenceTestCase: expected_hits_and_misses: tuple[tuple[int, int], ...] +@dataclass(frozen=True) +class NamedSelectorIntegrationTestCase: + """One compile of the DuckShop named-selector fixture under a `[rules]` table.""" + + description: str + rules_toml: str + expected_exit_code: int + expected_findings: tuple[tuple[str, str | None, int | None, int | None], ...] + expected_text: str = "" + + @dataclass(frozen=True) class PythonRulesIntegrationTestCase: """One compile of a DuckShop Python-rules fixture under a `[rules]` table.""" diff --git a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py index f161d036b..44970058e 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/helpers.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/helpers.py @@ -1357,6 +1357,19 @@ def _default_interrupt_handling() -> None: PYTHON_RULES_FIXTURES: Path = Path("benchmarks/duckshop/edge_cases/python_rules") +NAMED_SELECTOR_FIXTURE: Path = Path("benchmarks/duckshop/edge_cases/named_selectors") + + +def write_named_selector_project(*, project_dir: Path, rules_toml: str) -> None: + """Copy the DuckShop named-selector fixture under the given `[rules]` table.""" + + shutil.copytree(NAMED_SELECTOR_FIXTURE, project_dir, dirs_exist_ok=True) + write_project_files( + project_dir=project_dir, + files={ + "sqlbuild_project.toml": f'name = "orders"\nadapter = "duckdb"\n\n[rules]\n{rules_toml}' + }, + ) def write_python_rules_project(*, project_dir: Path, fixture: str, rules_toml: str) -> None: diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py index 3b3cad7f4..4091b3d81 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_rules.py @@ -34,6 +34,7 @@ EmptyRuleSelectionTestCase, ExplicitContractOutputRuleIntegrationTestCase, ImplicitAliasRuleIntegrationTestCase, + NamedSelectorIntegrationTestCase, NumericRangeDecisionIntegrationTestCase, ProjectRuleScratchStateTestCase, PythonRuleCacheSequenceTestCase, @@ -53,6 +54,7 @@ interrupt_main_after, observe_hosts, write_custom_host_project, + write_named_selector_project, write_project_files, write_project_scratch_rules, write_python_rules_project, @@ -93,6 +95,12 @@ def final_directory(*, model: Model, ctx: RuleContext) -> list[Finding]: _PYTHON_CACHE_TOML: str = ( 'name = "orders"\nadapter = "duckdb"\n\n[rules]\nselect = ["SQBRPYTHON101"]\n' ) +_NAMED_SELECTOR_SELECT: str = 'select = ["SQBRORCHESTRATION"]\n' +_NAMED_SELECTOR_EXCEPTION: str = ( + '\n[[rules.rule_exceptions]]\nrule = "SQBRORCHESTRATION101"\n' + 'path = "orchestration/jobs/orders_repair_job.toml"\n' + 'reason = "Rebuilds the four orders models after a late payments load."\n' +) _CUSTOM_CACHE_MODEL_SQL: str = 'MODEL (description "Orders");\nSELECT 1 AS order_id\n' _CUSTOM_CACHE_EDITED_SQL: str = _CUSTOM_CACHE_MODEL_SQL.replace("SELECT 1", "SELECT 2") @@ -3542,3 +3550,123 @@ def test_given_python_rule_cache_when_inputs_change_then_only_changed_files_reru assert tuple(observed) == test_case.expected_hits_and_misses assert tuple(cached) == test_case.expected_python_cache + + +@pytest.mark.parametrize( + "test_case", + [ + NamedSelectorIntegrationTestCase( + description=( + "listed jobs and sensor conditions over the limit, bound or imported, and computed " + "sensor conditions fail; folders, globs, job names and platform events pass" + ), + rules_toml=_NAMED_SELECTOR_SELECT, + expected_exit_code=1, + expected_findings=( + ("SQBRORCHESTRATION101", "orchestration/jobs/orders_listed_job.toml", 4, 10), + ("SQBRORCHESTRATION101", "orchestration/jobs/orders_repair_job.toml", 4, 10), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_built_sensor.py", + 10, + 10, + ), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_computed_sensor.py", + 10, + 28, + ), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_failure_sensor.py", + 6, + 12, + ), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_landed_sensor.py", + 10, + 10, + ), + ), + expected_text="job 'orders_listed_job' names 4 nodes in its selection (limit 3)", + ), + NamedSelectorIntegrationTestCase( + description="an exact rule exception keeps a deliberate list", + rules_toml=_NAMED_SELECTOR_SELECT + _NAMED_SELECTOR_EXCEPTION, + expected_exit_code=1, + expected_findings=( + ("SQBRORCHESTRATION101", "orchestration/jobs/orders_listed_job.toml", 4, 10), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_built_sensor.py", + 10, + 10, + ), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_computed_sensor.py", + 10, + 28, + ), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_failure_sensor.py", + 6, + 12, + ), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_landed_sensor.py", + 10, + 10, + ), + ), + expected_text="sensor 'on_orders_failure_sensor' names 4 nodes in its selection", + ), + NamedSelectorIntegrationTestCase( + description="a raised limit passes every list but still reports computed conditions", + rules_toml=( + _NAMED_SELECTOR_SELECT + + "\n[rules.rule_options.SQBRORCHESTRATION101]\nmax_named_selectors = 4\n" + ), + expected_exit_code=1, + expected_findings=( + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_built_sensor.py", + 10, + 10, + ), + ( + "SQBRORCHESTRATION101", + "orchestration/sensors/on_orders_computed_sensor.py", + 10, + 28, + ), + ), + expected_text="`landed_after(...)` is a function call", + ), + ], + ids=lambda case: case.description, +) +def test_given_orchestration_selections_when_compiling_then_named_selector_limit_follows_configuration( + test_case: NamedSelectorIntegrationTestCase, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + write_named_selector_project(project_dir=tmp_path, rules_toml=test_case.rules_toml) + + exit_code: int = main(["--project-dir", str(tmp_path), "compile", "--json", "--no-cache"]) + diagnostics: list[dict[str, Any]] = json.loads(capsys.readouterr().out)["diagnostics"] + + assert exit_code == test_case.expected_exit_code + assert ( + tuple( + (item["code"], item.get("path"), item.get("line"), item.get("column")) + for item in diagnostics + ) + == test_case.expected_findings + ) + assert test_case.expected_text in json.dumps(diagnostics) diff --git a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py index 803a5d1ad..084e9ade6 100644 --- a/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py +++ b/tests/unit/src/sqlbuild/rule_engine/_helpers/engine/test_catalogue.py @@ -44,6 +44,7 @@ "SQBRGRAPH101", "SQBRGRAPH102", "SQBRGRAPH103", + "SQBRORCHESTRATION101", "SQBRPROJECT101", "SQBRPROJECT102", "SQBRPROJECT103", @@ -111,6 +112,7 @@ "SQBRGRAPH101", "SQBRGRAPH102", "SQBRGRAPH103", + "SQBRORCHESTRATION101", "SQBRPROJECT101", "SQBRPROJECT102", "SQBRPROJECT103", diff --git a/website/src/content/docs/docs/concepts/orchestration.mdx b/website/src/content/docs/docs/concepts/orchestration.mdx index 3aee7cb79..61c8651bd 100644 --- a/website/src/content/docs/docs/concepts/orchestration.mdx +++ b/website/src/content/docs/docs/concepts/orchestration.mdx @@ -80,6 +80,10 @@ cpus = 2 A job's `[settings]` cannot set `select` or `exclude`: the job's own selection is the run's selection. +Select a pipeline by its folder or a tag rather than listing its nodes. Rule `SQBRORCHESTRATION101` +reports a job or sensor `select` that names more than three nodes one by one; see +[Orchestration Rules](/docs/concepts/rules/orchestration-rules). + ## Group jobs A job group runs its jobs as one run. A job waits for another job in the group when it reads a node diff --git a/website/src/content/docs/docs/concepts/rules.mdx b/website/src/content/docs/docs/concepts/rules.mdx index 11cf81241..c590d7e61 100644 --- a/website/src/content/docs/docs/concepts/rules.mdx +++ b/website/src/content/docs/docs/concepts/rules.mdx @@ -150,7 +150,9 @@ SQLBuild maintains native built-in Rules for reusable requirements. SQL Rule cod `SQLCLEAN`, for example `SQLCLEAN004`. Rules that read the project, such as `SQBRGRAPH101`, begin with `SQBR`. The `SQBRPYTHON` Rules check the project's Python: annotations, imports, function shape and the layout of `python/`. They run only when `select` names `SQBRPYTHON` or one of its -codes; see [Python Rules](/docs/concepts/rules/python-rules). +codes; see [Python Rules](/docs/concepts/rules/python-rules). `SQBRORCHESTRATION101` keeps job and +sensor selections from listing nodes one by one; see +[Orchestration Rules](/docs/concepts/rules/orchestration-rules). Projects can define custom Python Rules under `rules/**/*.py`. Their codes begin with `XSQBR`, for example `XSQBRARCH001`. Custom Rules use the same selection, findings, exceptions, execution order, diff --git a/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx b/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx index 3fb0dc23d..50e70594d 100644 --- a/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx +++ b/website/src/content/docs/docs/concepts/rules/configuration-and-selection.mdx @@ -81,6 +81,9 @@ optional `domain`; `qualifier` and `paths` accept globs. See The Python Rules take whole-number limits such as `max_arguments` for `SQBRPYTHON504`; see [Python Rules](/docs/concepts/rules/python-rules#rule-options). +`SQBRORCHESTRATION101` accepts `max_named_selectors`, the most nodes one job or sensor selection may +name one by one (default 3); see [Orchestration Rules](/docs/concepts/rules/orchestration-rules). + Unknown Rule codes, option names, and invalid option values fail configuration. ## Focused selection diff --git a/website/src/content/docs/docs/concepts/rules/orchestration-rules.mdx b/website/src/content/docs/docs/concepts/rules/orchestration-rules.mdx new file mode 100644 index 000000000..1e5038ede --- /dev/null +++ b/website/src/content/docs/docs/concepts/rules/orchestration-rules.mdx @@ -0,0 +1,92 @@ +--- +title: "Orchestration Rules" +description: "Keep job and sensor selections from listing nodes one by one." +--- + +`SQBRORCHESTRATION101` reports a job or sensor whose `select` names more than three nodes one by one. +Select a pipeline by its folder or a tag instead, so nodes added to it later are built too. + +```toml +# orchestration/jobs/nightly_orders_job.toml +name = "nightly_orders_job" +description = "Orders and their daily totals" +select = "stg_orders stg_payments int_orders order_totals" +``` + +```text +error[SQBRORCHESTRATION101]: job 'nightly_orders_job' names 4 nodes in its selection (limit 3) + --> orchestration/jobs/nightly_orders_job.toml:3:10 + = help: If these nodes are one pipeline, select its folder or a tag. If the job deliberately targets exactly these nodes, add an exact rule exception for this file with a reason. +``` + +```toml +select = "models/orders" +``` + +The Rule is selected with `SQBR` or `SQBRORCHESTRATION`, and with its exact code. + +## What counts as a named node + +The Rule reads the `select` of every [job](/docs/concepts/orchestration#declare-a-job) and the +`select` of each `node_event`, `source_new_data` and `source_stale` condition of a +[sensor](/docs/concepts/orchestration#start-a-job-from-a-sensor). It counts the distinct +[selector](/docs/concepts/selectors) atoms that each name one node. + +| Atom | Counts | +| --- | --- | +| A bare name, such as `order_totals` | Yes | +| A typed name, such as `source:raw_orders` or `task:refund_review` | Yes | +| Each name in an intersection, such as `order_totals,tag:finance` | The name only | +| `tag:`, a folder path, `job:`, a job or job group name | No | +| A name with `+` before or after it | No | +| A glob, such as `fct_*` | No | + +A name written twice counts once. A job's `exclude` is not checked. + +## Change the limit + +Set `max_named_selectors` to the most nodes one selection may name. The default is 3, and 0 rejects +every named node. + +```toml +[rules.rule_options.SQBRORCHESTRATION101] +max_named_selectors = 5 +``` + +## Keep a deliberate list + +A repair job that targets exactly a few nodes is a deliberate exception. Record it for the job's +file with a reason: + +```toml +[[rules.rule_exceptions]] +rule = "SQBRORCHESTRATION101" +path = "orchestration/jobs/repair_orders_job.toml" +reason = "Rebuilds the four order models after a late payments load." +``` + +See [Findings and exceptions](/docs/concepts/rules/findings-and-exceptions#exact-exceptions). + +## Sensor conditions the Rule cannot count + +The Rule reads a sensor's `when` without running the module. It follows a name bound at module +level, in the sensor's file or in a project module the sensor imports it from, to the condition +call that name holds. + +| `when` value | Result | +| --- | --- | +| A selecting condition whose `select` is a quoted string | Counted | +| A `platform_event` or `run_event` condition, written inline or held in a name | Not checked | +| A selecting condition whose `select` is a variable or an expression | Reported | +| A call to any other function, such as a helper that builds the condition | Reported | +| A name imported from outside the project, or not bound at module level | Reported | +| Any other expression, such as `module.NAME` | Reported | + +A reported condition names what the Rule could not resolve: + +```text +error[SQBRORCHESTRATION101]: sensor 'on_orders_built_sensor' has a condition the rule cannot resolve: `landed_after(...)` is a function call, so the nodes it selects cannot be counted +``` + +To keep a condition the Rule cannot count, add an exact rule exception for the sensor's file with a +reason. diff --git a/website/src/sidebar.json b/website/src/sidebar.json index 3d279d3ec..4312a3d73 100644 --- a/website/src/sidebar.json +++ b/website/src/sidebar.json @@ -76,6 +76,7 @@ "docs/concepts/rules/configuration-and-selection", "docs/concepts/rules/findings-and-exceptions", "docs/concepts/rules/python-rules", + "docs/concepts/rules/orchestration-rules", "docs/concepts/rules/execution-and-caching" ] }