Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor detached tests implementation to group based on test parents #1469

Open
tatiana opened this issue Jan 15, 2025 · 0 comments
Open

Refactor detached tests implementation to group based on test parents #1469

tatiana opened this issue Jan 15, 2025 · 0 comments
Labels
dbt:test Primarily related to dbt test command or functionality
Milestone

Comments

@tatiana
Copy link
Collaborator

tatiana commented Jan 15, 2025

Since Cosmos 1.8.1, particularly change #1433, if a test has multiple parents, we create an independent test task for each.
Our original approach was to name the task based on the test name, which led to issue #1440.

This bug affected many users, so we released a quick fix, #1464, that would change the test name to "detached_{incremental unique number}_test" if the name were longer than 250 characters. This implementation, however, relies on an internal Airflow implementation detail: that a new process always parses the DAG, as described in https://github.com/astronomer/astronomer-cosmos/pull/1464/files#r1916648300. This is true for Airflow 2.x and should remain True in Airflow 3.0 - but it is an internal implementation detail.

The goal with this ticket is to refactor the detached test implementation so we only have a single detached task for each group of parents. This way, we could name the task either:
a) "parent1_parent2_..._test"
b) or "<deterministic_hash([parent1, parent2])>_test", if (a) is bigger than 250 chars

For further context, check discussion: https://github.com/astronomer/astronomer-cosmos/pull/1464/files#r1916450202

@tatiana tatiana added this to the Cosmos 1.9.0 milestone Jan 15, 2025
@dosubot dosubot bot added dbt:test Primarily related to dbt test command or functionality labels Jan 15, 2025
tatiana added a commit that referenced this issue Jan 15, 2025
Since we introduced detached test tasks to fix a customer issue in #1433
(release 1.8.1), we changed how Cosmos renders DAGs, with the chance
that Cosmos significantly changed how it renders a dbt project - even
when users did not change their `DbtDag` or `DbtTaskGroup`
configuration. This is unacceptable in a micro release - and for this
reason we're reverting this change and making the feature opt-in.

PR #1433 led to issues such as #1464, reported by multiple Cosmos users,
and also issues that did not become Github issues, such as an Astro
customer who reported that when they upgraded to Cosmos 1.8.1, the
number of tasks increased dramatically. One problem with #1433 was that
it did not empower users to opt in or out of having detached test nodes,
solving the problem for some but causing problems for many.

This PR aims to solve this problem by introducing a new property to
`RenderConfig`: `should_detach_multiple_parents_tests`. We are reverting
the Cosmos DAG rendering to what it was in 1.8.0 and before: by default,
it will not detach tests with multiple parents. Users must opt-in for
this behaviour if and when they want to.

We understand this may be perceived as a breaking change by some, but it
is the correct way to move forward and avoid causing further disruption.

We are planning to review the current implementation, as described in
#1469. For now, this PR documents the current behaviour and
empowers users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dbt:test Primarily related to dbt test command or functionality
Projects
None yet
Development

No branches or pull requests

1 participant