Support use of YAML selectors when using LoadMode.DBT_MANIFEST

[YourRoyalLinus]

Description

Add support for the use of yaml selectors when loading dbt projects using LoadMode.DBT_MANIFEST. This works by parsing the YAML selectors found in the manifest file that have already been processed by the dbt parser into corresponding select and exclude selections that are passed onto the select_nodes() function.

The performance overhead for the initial parse will depend on the user's selector.yaml definition, but in testing it has been reasonable. The parsing penalty is only paid once, as future selector to select/exclude mapping access is done from the cache, if enabled (similar to dbt_ls_cache).

Key Changes

• Implemented a parser to convert full YAML selector definitions into corresponding select and exclude selections
  • Is only used for graphs with RenderConfig.load_method = DBT_MANIFEST and a defined RenderConfig.selector
• Implemented cache behavior to store and retrieve the parsed YAML selector definitions
  • Added a new enable_cache_yaml_selectors cosmos setting to enable/disable caching
  • Invalidates if YamlSelector implementation change
    • Ensures users do not have to manually clear the cache if the spec is changed
• Renamed dbt_ls_cache to be more general so it can be used to store parsed selector yaml definitions
  • Cache should always be mutually exclusive for the dbt_ls output or parsed selector yaml

Limitations

• I did not implement parser support for the indirect_selection or default keywords.
  • It seems plausible that both of these can be implemented if desired. Omitting them was done to limit scope.
• This is not a full YAML selector parser, akin to what dbt implements.
  • While much of the implementation is borrowed from the latest version of dbt core, this expects the selector definitions to be in the state that exists in an unmodified manifest file.
    • This approach allows the Cosmos parser to only to handle fully structured method-value selection definitions.
    • I kept many high-level parser exceptions in the Cosmos implementation that are technically redundant with the dbt parser
      • This was done to try and catch user-modified manifest files to fail fast with helpful error messages.
      • Modifying the selector definitions found in the manifest file is considered undefined behavior.

These limitations are reflected in the documentation.

Related Issue(s)

Closes #2257

Breaking Change?

• I reused the existing dbt_ls cache cache key and functionality, generalizing where possible
  • This is an implementation detail and should not break the public API nor alter any existing behavior
  • This requires it to be impossible to have a dbt_ls and yaml_selectors cache at the same time due
    • This is technically possible by setting the DbtGraph.cache_identifier to be the same, but this can't be configured by a user.

These are reflected in the documentation.

Checklist

• I have made corresponding changes to the documentation (if required)
• I have added tests that prove my fix is effective or that my feature works

[netlify]

✅ Deploy Preview for astronomer-cosmos canceled.

Name	Link
🔨 Latest commit	9cdf03e
🔍 Latest deploy log	https://app.netlify.com/projects/astronomer-cosmos/deploys/697b6fa3c4b2fe0008ddbce0

[YourRoyalLinus]

I don't think the current state of this PR is ready for merge. I wanted to get feedback on this approach before finalizing. I have unit tests from the implementation I manage that I will port over - along with writing additional cosmos-specific tests - before the PR would moved out of draft.

Modifications to existing code, as well as how I composed my changes, were mostly done in a non-intrusive way to appease the linter. I'm fully open to alternative approaches.

This is my first OSS PR. If I've committed any faux pas, please let me know

[tatiana]

This is a very exciting feature, @YourRoyalLinus. Thanks for working on it!

[tatiana]

Hi @YourRoyalLinus, thanks a lot for all the tests and updates on this. We really appreciate it.
We'll update the manifest.json files in the remote locations to include the selectors and re-run the integration tests.
I'm still hopeful we can fit this in the 1.13.0 release, aimed for tomorrow. It's a very significant feature which will benefit many people.

[tatiana]

@YourRoyalLinus, I had a quick look at the tests and tried to fix them in a separate branch (#2296), which is a draft PR so the CI can trigger the job. I'll close it once the checks in your branch are passing.

(1) Some integration tests were failing with:

    ERROR tests/test_example_dags.py - AssertionError: assert not {'/home/runner/work/astronomer-cosmos/astronomer-cosmos/dev/dags/cosmos_manifest_selectors_example.py': 'Traceback (mo...he required Object Storage feature is unavailable in Airflow version 2.7.0. Please upgrade to Airflow 2.8 or later.\n'}
     +  where {'/home/runner/work/astronomer-cosmos/astronomer-cosmos/dev/dags/cosmos_manifest_selectors_example.py': 'Traceback (mo...he required Object Storage feature is unavailable in Airflow version 2.7.0. Please upgrade to Airflow 2.8 or later.\n'} = <airflow.models.dagbag.DagBag object at 0x7fa3c6e724d0>.import_errors
    ERROR tests/test_example_dags_no_connections.py - AssertionError: assert not {'/home/runner/work/astronomer-cosmos/astronomer-cosmos/dev/dags/cosmos_manifest_selectors_example.py': 'Traceback (mo...he required Object Storage feature is unavailable in Airflow version 2.7.0. Please upgrade to Airflow 2.8 or later.\n'}
     +  where {'/home/runner/work/astronomer-cosmos/astronomer-cosmos/dev/dags/cosmos_manifest_selectors_example.py': 'Traceback (mo...he required Object Storage feature is unavailable in Airflow version 2.7.0. Please upgrade to Airflow 2.8 or later.\n'} = <airflow.models.dagbag.DagBag object at 0x7fa3be1d89d0>.import_errors

The reason is "Object Storage feature is unavailable in Airflow version 2.7.0. Please upgrade to Airflow 2.8 or later". This is how you can fix it: 424a9a1

(2) dbt Fusion tests stopped working because the default selector did not select any nodes in the jaffle_shop dbt project

There were dbt Fusion tests that referenced the dbt project jaffle_shop that you changed and would actually run the DAG and check the DAG topology - including which nodes were rendered.

Since we set the default selector to not match any nodes, this behaviour changed:

=========================== short test summary info ============================
FAILED tests/test_dbtf.py::test_dbt_dag_with_dbt_fusion - assert 0 == 23
 +  where 0 = len({})
 +    where {} = <cosmos.dbt.graph.DbtGraph object at 0x7f0b68bb80d0>.filtered_nodes
 +      where <cosmos.dbt.graph.DbtGraph object at 0x7f0b68bb80d0> = <DAG: snowflake_dbt_fusion_dag>.dbt_graph

My advice is that we do not add the selector changes to dev/dags/dbt/jaffle_shop, but instead to dev/dags/dbt/altered_jaffle_shop. Another suggestion is that we do not set the default selector to not match any nodes.

(3) The DAG cosmos_manifest_selectors_examples.py failed to run in a clean database because it would attempt to run the dbt model customers, without having run its upstream tasks. This can be observed in https://github.com/astronomer/astronomer-cosmos/actions/runs/21476188465/job/61860709231?pr=2296

I fixed all the issues in #2296. If you get a chance, please add these fixes to this PR - otherwise we may merge your PR as it is, and then merge the fixes.

[tatiana]

There are significant changes in this PR, which may lead to issues. That said, given the changes are relatively isolated, since they mostly relate to adding support to selector when using LoadMode.DBT_MANIFEST, we decided to go ahead with merging and releasing this in Cosmos 1.13.0.

Even though the tests are not passing in this branch, we managed to fix them in #2296. We decided to go ahead and merge this PR as-is, since we'll merge the test fixes straight after.

[YourRoyalLinus]

Hi @YourRoyalLinus, thanks a lot for all the tests and updates on this. We really appreciate it. We'll update the manifest.json files in the remote locations to include the selectors and re-run the integration tests. I'm still hopeful we can fit this in the 1.13.0 release, aimed for tomorrow. It's a very significant feature which will benefit many people.

HI @tatiana, this is great! Happy to be able to contribute back. My company will likely be the first user of this feature once its live, so if there's anything that got missed I should hopefully be the first to find (and fix) it.

[tatiana]

@YourRoyalLinus that's awesome!

@pankajastro just released 1.13.0a4 after we merged your PR - please, feel free to try it out and give us any feedback:
https://github.com/astronomer/astronomer-cosmos/releases/tag/astronomer-cosmos-v1.13.0a4

We're aiming to release the stable version tomorrow, GMT afternoon.

[YourRoyalLinus]

@YourRoyalLinus that's awesome!

@pankajastro just released 1.13.0a4 after we merged your PR - please, feel free to try it out and give us any feedback: https://github.com/astronomer/astronomer-cosmos/releases/tag/astronomer-cosmos-v1.13.0a4

We're aiming to release the stable version tomorrow, GMT afternoon.

Hi @tatiana,

I was able to use the RC branch to test changes on our deployment in a dev environment and everything rendered as expected!

One thing I did notice during my QA was that one bad selector configuration will cause a dag parse error for all dbt dags using LoadMode.DBT_MANIFEST with selectors. A more ideal state is that only DAGs referencing an invalid selector should throw an error during parsing. This was something I had a fix for in the version I maintained. I'm looking to see if it's feasible to port those changes over as well. If possible, this should be a low level of effort. Once I have it scoped I will create an issue/PR.

If you have any thoughts or objections, please let me know.

[tatiana]

Thanks a lot @YourRoyalLinus - we'll appreciate any further improvements in the feature. Have a great weekend!