Skip to content

Commit

Permalink
fix(query): remove engine validation for query (#9271)
Browse files Browse the repository at this point in the history
### Description

We don't want to validate certain issues with graphs with `turbo query`,
because it should be still usable even with an invalid graph.

### Testing Instructions

Added a test that shows that `turbo query` is tolerant of issues that
`turbo run` is not.
  • Loading branch information
NicholasLYang authored Oct 16, 2024
1 parent 2a7c317 commit 7cb4f75
Show file tree
Hide file tree
Showing 12 changed files with 269 additions and 3 deletions.
4 changes: 3 additions & 1 deletion crates/turborepo-lib/src/commands/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ pub async fn run(
execution_args: Box::default(),
});

let run_builder = RunBuilder::new(base)?.add_all_tasks();
let run_builder = RunBuilder::new(base)?
.add_all_tasks()
.do_not_validate_engine();
let run = run_builder.build(&handler, telemetry).await?;

if let Some(query) = query {
Expand Down
9 changes: 8 additions & 1 deletion crates/turborepo-lib/src/engine/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ pub struct EngineBuilder<'a> {
root_enabled_tasks: HashSet<TaskName<'static>>,
tasks_only: bool,
add_all_tasks: bool,
should_validate_engine: bool,
}

impl<'a> EngineBuilder<'a> {
Expand All @@ -120,6 +121,7 @@ impl<'a> EngineBuilder<'a> {
root_enabled_tasks: HashSet::new(),
tasks_only: false,
add_all_tasks: false,
should_validate_engine: true,
}
}

Expand Down Expand Up @@ -157,6 +159,11 @@ impl<'a> EngineBuilder<'a> {
self
}

pub fn do_not_validate_engine(mut self) -> Self {
self.should_validate_engine = false;
self
}

// Returns the set of allowed tasks that can be run if --only is used
// The set is exactly the product of the packages in filter and tasks specified
// by CLI
Expand Down Expand Up @@ -502,7 +509,7 @@ impl<'a> EngineBuilder<'a> {
}
}

if task_definitions.is_empty() {
if task_definitions.is_empty() && self.should_validate_engine {
let (span, text) = task_id.span_and_text("turbo.json");
return Err(Error::MissingPackageTask {
span,
Expand Down
14 changes: 13 additions & 1 deletion crates/turborepo-lib/src/run/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ pub struct RunBuilder {
should_print_prelude_override: Option<bool>,
allow_missing_package_manager: bool,
allow_no_turbo_json: bool,
// In query, we don't want to validate the engine. Defaults to `true`
should_validate_engine: bool,
// If true, we will add all tasks to the graph, even if they are not specified
add_all_tasks: bool,
}
Expand Down Expand Up @@ -111,6 +113,7 @@ impl RunBuilder {
allow_missing_package_manager,
root_turbo_json_path,
allow_no_turbo_json,
should_validate_engine: true,
add_all_tasks: false,
})
}
Expand All @@ -130,6 +133,11 @@ impl RunBuilder {
self
}

pub fn do_not_validate_engine(mut self) -> Self {
self.should_validate_engine = false;
self
}

fn connect_process_manager(&self, signal_subscriber: SignalSubscriber) {
let manager = self.processes.clone();
tokio::spawn(async move {
Expand Down Expand Up @@ -496,6 +504,10 @@ impl RunBuilder {
builder = builder.add_all_tasks();
}

if !self.should_validate_engine {
builder = builder.do_not_validate_engine();
}

let mut engine = builder.build()?;

// If we have an initial task, we prune out the engine to only
Expand All @@ -504,7 +516,7 @@ impl RunBuilder {
engine = engine.create_engine_for_subgraph(entrypoint_packages);
}

if !self.opts.run_opts.parallel {
if !self.opts.run_opts.parallel && self.should_validate_engine {
engine
.validate(
pkg_dep_graph,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "app-a",
"scripts": {
"build": "echo 'build app-a'",
"test": "echo 'test app-a'"
},
"dependencies": {
"lib-a": "*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "app-b",
"scripts": {
"build": "echo 'build app-b'",
"test": "echo 'test app-b'"
},
"dependencies": {
"lib-b": "*",
"lib-c": "*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "lib-a",
"scripts": {
"build": "echo 'build lib-a'",
"test": "echo 'test lib-a'"
},
"dependencies": {
"lib-b": "*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "lib-b",
"scripts": {
"build": "echo 'build lib-b'",
"test": "echo 'test lib-b'"
},
"dependencies": {
"lib-d": "*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "lib-c",
"scripts": {
"build": "echo 'build lib-c'"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "lib-d",
"scripts": {
"build": "echo 'build lib-d'",
"test": "echo 'test lib-d'"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "unknown-dependency",
"workspaces": [
"app-a",
"app-b",
"lib-a",
"lib-b",
"lib-c",
"lib-d"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"tasks": {
"build0": {
"dependsOn": [
"^build0",
"prepare"
]
},
"test": {
"dependsOn": [
"^build0",
"prepare"
]
},
"prepare": {},
"side-quest": {
"dependsOn": [
"prepare"
]
},
"build1": {
"dependsOn": [
"^build1"
]
},
"build2": {
"dependsOn": [
"app-a#custom"
]
}
}
}
148 changes: 148 additions & 0 deletions turborepo-tests/integration/tests/query/validation.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
Setup
$ . ${TESTDIR}/../../../helpers/setup_integration_test.sh persistent_dependencies/10-too-many

Validate that we get an error when we try to run multiple persistent tasks with concurrency 1
$ ${TURBO} run build --concurrency=1
x invalid task configuration

Error: x You have 2 persistent tasks but `turbo` is configured for concurrency of
| 1. Set --concurrency to at least 3

[1]

However on query, we ignore this validation
$ ${TURBO} query "query { packages { items { tasks { items { fullName } } } } }" | jq
WARNING query command is experimental and may change in the future
{
"data": {
"packages": {
"items": [
{
"tasks": {
"items": []
}
},
{
"tasks": {
"items": [
{
"fullName": "one#build"
}
]
}
},
{
"tasks": {
"items": [
{
"fullName": "two#build"
}
]
}
}
]
}
}
}

Setup
$ . ${TESTDIR}/../../../helpers/setup_integration_test.sh task_dependencies/invalid-dependency
warning: re-init: ignored --initial-branch=main

Validate that we get an error when trying to depend on a task that doesn't exist
$ ${TURBO} run build2
x Could not find "app-a#custom" in root turbo.json or "custom" in package
,-[turbo.json:27:1]
27 | "dependsOn": [
28 | "app-a#custom"
: ^^^^^^^^^^^^^^
29 | ]
`----

[1]

However, we don't get an error when we query
$ ${TURBO} query "query { packages { items { tasks { items { fullName } } } } }" | jq
WARNING query command is experimental and may change in the future
{
"data": {
"packages": {
"items": [
{
"tasks": {
"items": []
}
},
{
"tasks": {
"items": [
{
"fullName": "app-a#build"
},
{
"fullName": "app-a#test"
}
]
}
},
{
"tasks": {
"items": [
{
"fullName": "app-b#build"
},
{
"fullName": "app-b#test"
}
]
}
},
{
"tasks": {
"items": [
{
"fullName": "lib-a#build"
},
{
"fullName": "lib-a#test"
}
]
}
},
{
"tasks": {
"items": [
{
"fullName": "lib-b#build"
},
{
"fullName": "lib-b#test"
}
]
}
},
{
"tasks": {
"items": [
{
"fullName": "lib-c#build"
}
]
}
},
{
"tasks": {
"items": [
{
"fullName": "lib-d#build"
},
{
"fullName": "lib-d#test"
}
]
}
}
]
}
}
}

0 comments on commit 7cb4f75

Please sign in to comment.