Additions to docs regarding the DBT_MANIFEST load mode

[dwreeves]

Description

I thought that there were a few aspects regarding execution modes that could require more clarification in the docs.

• The parsing methods docs mentions that only LOCAL execution mode is supported for DBT_LS, but the reverse was not true (i.e. execution mode docs made no mention of parsing methods), so I added notes about that.
• GCC docs suggest using VIRTUALENV execution mode, but makes no mention of the fact that the DBT_LS parsing method is not supported in this execution mode. Naturally, in this case, users should be utilizing the DBT_MANIFEST load mode, but that means that the docs are incomplete since they don't include a manifest_path=? in the ProjectConfig.
  • Note that there are also discussions in the Airflow Slack regarding issues users have had parsing the DbtDag in GCC that are fixable via using a pre-compiled manifest,json, e.g. https://apache-airflow.slack.com/archives/C059CC42E9W/p1696435273519979
  • Also see Evaluate alternatives to rendering dbt projects other than during DAG parsing time #520 for more discussion.
• Generally speaking when doing the DBT_MANIFEST load method, the pattern is that you run dbt deps && dbt compile as part of your deployment, and upload your full dbt project including these artifacts. This deployment approach may be obvious to veteran users of Airflow and/or dbt, but it may not be obvious to everyone, so I think adding a couple sentences in parsing-methods.rst is beneficial.

Related Issue(s)

Not explicitly related, but #520 discusses some issues encountered using the default parsing method. (Specifically, running dbt deps from a blank slate tends to slow everything down a lot.)

Part of my motivation for adding to the docs is to better advertise + better document this alternate method of parsing the dbt DAG.

Breaking Change?

n/a

Checklist

n/a

[netlify]

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name	Link
🔨 Latest commit	66d9f36
🔍 Latest deploy log	https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/6592f0f6f82e5c00086b999d

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d787616) 93.28% compared to head (e4b16a2) 93.28%.

❗ Current head e4b16a2 differs from pull request most recent head 72dab9b. Consider uploading reports for the commit 72dab9b to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #757   +/-   ##
=======================================
  Coverage   93.28%   93.28%           
=======================================
  Files          55       55           
  Lines        2502     2502           
=======================================
  Hits         2334     2334           
  Misses        168      168           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tatiana]

Thanks for improving the docs, @dwreeves - some minor comments inline

[dwreeves]

@tatiana Some context: I was basing this and all other documentation changes I've proposed on two things:

(1) the code that currently exists in cosmos/dbt/graph.py here is that it will not automatically execute load_via_dbt_ls() if the execution mode is LOCAL.

(2) The docs already mention my wording change somewhere else:

I think I see what your changes are attempting to do. But before I continue with that train of thought, I am wondering: Is it possible that execution_mode == ExecutionMode.LOCAL should not be part of the predicate that checks whether DBT_LS mode is run? I think our divergence of views here makes more sense in a world where that execution_mode == ExecutionMode.LOCAL check is not part of cosmos.dbt.graph.DbtGraph.load. If that checks out, then I suppose I just may be confused about something.

[dwreeves]

To add to that, if that does check out, then I think my confusion would be that you are viewing the AUTOMATIC and DBT_LS as separate modes, whereas I'm treating AUTOMATIC as a way to access DBT_LS. In that case where AUTOMATIC is viewed as its own thing, I still think:

• the docs should make some note that AUTOMATIC will not use dbt ls to parse if the execution mode is local. (Although, you are right that "it is not supported" is too strong a statement; the more accurate statement would be to describe this in terms of how the AUTOMATIC load mode works.)
• Without a manifest.json specified, the GCC setup will default to the LoadMode.CUSTOM logic. Perhaps that is fine and my suggestion to specify a manifest is off-base, but I also believe that's an accurate description of the current behavior of the existing GCC suggested setup. So I guess I would then ask: should LoadMode.DBT_MANIFEST be preferred over LoadMode.CUSTOM, and/or should the GCC docs mention anything about the default use of LoadMode.CUSTOM?
• The statement inside parsing-methods.rst should be relaxed from saying: "This only works for the local execution mode" and should instead say: "This only works if a dbt command is available to the scheduler."

WDYT?

[tatiana]

Hey @dwreeves, first of all, thank you very much for working on this. We've been struggling to keep docs up-to-date, and they are as important as the code - directly affecting the adoption of Cosmos.

This particular discussion is also relevant for us to review and potentially improve Cosmos' behaviour.

(1) the code that currently exists in cosmos/dbt/graph.py here is that it will not automatically execute load_via_dbt_ls() if the execution mode is LOCAL.

If the manifest file is not set and the user-defined a profile configuration, yes, LoadMode.AUTOMATIC will call load_via_dbt_ls() by default when users use ExecutionMode.LOCAL. From the users I've supported so far, many end up using this.

(2) The docs already mention my wording change somewhere else

This is incorrect; users can use LoadMode.DBT_LS regardless of the ExecutionMode choice.

Could you remove this note as part of this PR?

During the last few weeks, we have been detaching the configuration of rendering the DAG from how Cosmos tasks execute dbt, where possible (an example issue: #568). This is particularly relevant when users use ExecutionMode.Kubernetes since the "environment" where the dbt commands run diverges significantly from where the DAG is parsed.

I think I see what your changes are attempting to do. But before I continue with that train of thought, I am wondering: Is it possible that execution_mode == ExecutionMode.LOCAL should not be part of the predicate that checks whether DBT_LS mode is run?

I don't think so.

ExecutionMode.LOCAL can be used with any of the "four" DAG parsing modes (
LoadMode.AUTOMATIC , LoadMode.MANIFEST , LoadMode.DBT_LS and LoadMode.CUSTOM). We're expecting more parsing methods to be introduced, such as #733, which are also independent of how the DAG is rendered.

Additionally, LoadMode.DBT_LS can also be used with any of the supported execution modes (ExecutionMode.LOCAL, ExecutionMode.VIRTUALENV, ExecutionMode.DOCKER and ExecutionMode.KUBERNETES). The choice of how to render the DAG is independent of how the tasks execute the dbt command. Again, there are a few ways we could extend the current execution modes - and they should still work with LoadMode.DBT_LS. One approach, for instance, could be for us to introduce native Airflow execution modes that, for example, rely on dbt to compile the dbt project SQL but run the actual transformations using Airflow async operators.

I agree that usually users who opt to run the dbt commands using ExecutionMode.KUBERNETES will have a more complex CI/CD process and will likely want to "isolate" the dbt command from being run in their Airflow environment - very probably relying on LoadMode.MANIFEST (and soon LoadMode.DBT_LS_FILE) to parse their DAG - avoiding LoadMode.DBT_LS. But this is a choice, not a restriction imposed by Cosmos.

I think our divergence of views here makes more sense in a world where execution_mode == ExecutionMode.LOCAL check is not part of cosmos.dbt.graph.DbtGraph.load. If that checks out, then I suppose I just may be confused about something.

I addressed this before; please let me know if not.

To add to that, if that does check out, I think my confusion would be that you view the AUTOMATIC and DBT_LS as separate modes, whereas I'm treating AUTOMATIC as a way to access DBT_LS. In that case where AUTOMATIC is regarded as its own thing, I still think:

Yes, LoadMode.AUTOMATIC and LoadMode.DBT_LS are different. LoadMode.AUTOMATIC is the best effort to figure out how to parse the dbt project based on the user configuration; it can lead to any of the other three current load methods to be used (LoadMode.MANIFEST, LoadMode.DBT_LS and LoadMode.CUSTOM). If the user gives a manifest, that's what is used. If the user does not, and Cosmos has reasons to believe the dbt command is available locally, it will attempt to use dbt ls. If that doesn't work, it then uses the custom parsing method.

the docs should note that AUTOMATIC will not use dbt ls to parse if the execution mode is local. (Although you are right that "it is not supported" is too strong a statement, the more accurate statement would be to describe how the AUTOMATIC load mode works.)

I've thought about this, and perhaps to simplify; we should change the following:

astronomer-cosmos/cosmos/dbt/graph.py

Line 173 in 4369987

if execution_mode == ExecutionMode.LOCAL and self.profile_config:

To be:

                If self.profile_config:

This would simplify the docs and the explanation of LoadMode.AUTOMATIC. Since we're catching the exception, it should also be backwards compatible. WDYT? I can log a follow-up ticket for this - I don't think we should change the code as part of this docs update.

Without a manifest.json specified, the GCC setup will default to the LoadMode.CUSTOM logic. That may be fine, and my suggestion to establish a manifest is off-base. Still, I also believe that's an accurate description of the current behaviour of the existing GCC suggested setup. So I would then ask: should LoadMode.DBT_MANIFEST be preferred over LoadMode.CUSTOM, and should the GCC docs mention anything about the default use of LoadMode?CUSTOM?

I agree with your proposal. For GCC, as of Cosmos 1.2.x, I believe we should encourage users to set the manifest using ProjectConfig(manifest_path=...). It is a more accurate way of parsing the dbt project than fall backing to LoadMode.CUSTOM. From Cosmos 1.3.x, I'd recommend using the LoadMode.DBT_LS_FILE: #733.

Yes, I also agree we should highlight to GCC users that if they don't give a manifest, LoadMode.AUTOMATIC will fall back to using Cosmos' LoadMode.CUSTOM logic.

The statement inside parsing-methods. rst should be relaxed from saying: "This only works for the local execution mode", and should instead say: "This only works if a dbt command is available to the scheduler."

Yes, I like your suggestion! Better than just removing, as a previously mentioned.

[dwreeves]

I've made changes to the PR as per the discussion.

[netlify]

✅ Deploy Preview for sunny-pastelito-5ecb04 ready!

Name	Link
🔨 Latest commit	72dab9b
🔍 Latest deploy log	https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/65968ad792d7230008530333
😎 Deploy Preview	https://deploy-preview-757--sunny-pastelito-5ecb04.netlify.app/configuration/parsing-methods
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[tatiana]

Thanks for addressing all the feedback, @dwreeves , we'll release this as part of 1.3!