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

LoadMode.DBT_LS unnecessarily requires render_config.dbt_deps=True #602

Closed
nicohein opened this issue Oct 14, 2023 · 4 comments
Closed

LoadMode.DBT_LS unnecessarily requires render_config.dbt_deps=True #602

nicohein opened this issue Oct 14, 2023 · 4 comments
Labels
awaiting_response parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing

Comments

@nicohein
Copy link

With the version 1.2.0 I got the below error with the LoadMode.DBT_LS.

  File "/opt/python3.8/lib/python3.8/site-packages/cosmos/dbt/graph.py", line 134, in load
    load_method[method]()
  File "/opt/python3.8/lib/python3.8/site-packages/cosmos/dbt/graph.py", line 243, in load_via_dbt_ls
    raise CosmosLoadDbtException(
cosmos.dbt.graph.CosmosLoadDbtException: Unable to run dbt ls command due to missing dbt_packages. Set render_config.dbt_deps=True.

The below config was working with version 1.1.2.

RenderConfig(
    test_behavior=TestBehavior.AFTER_EACH,
    load_method=LoadMode.DBT_LS,
    dbt_deps=False,
)

There are no dependencies required for dbt_ls in my setup. I fully understand that there are cases but it's not universally true.
This enforced setting leads to a dag load times of between 45 and 70s (up from 25s) which I find very long with all our (more than 200) other DAGs loading below a single second.

My suggestion would be to throw a warning instead of an error: https://github.com/astronomer/astronomer-cosmos/blame/main/cosmos/dbt/graph.py#L244
I would understand the behavior if the dbt ls command was throwing an error but this is generated from stdout, hence the request.

@tatiana
Copy link
Collaborator

tatiana commented Oct 16, 2023

Hi @nicohein , thank you very much for the feedback!

I just reviewed the code that should be generating this exception:

if 'Run "dbt deps" to install package dependencies' in stdout:
raise CosmosLoadDbtException(
"Unable to run dbt ls command due to missing dbt_packages. Set render_config.dbt_deps=True."
)

And, ATM, this exception should only be raised if dbt itself outputs Run "dbt deps" to install package dependencies. Please, could you confirm if this is the case in your project?

It surprises me that Cosmos will raise this exception if you set render_config.dbt_deps=False and the project has no dependencies. Can you give a reproducible example of the issue?

There may be an issue with the logic somewhere, but if dbt complains about dependencies not being installed - the other dbt commands will not work. Cosmos has been raising exceptions in this scenario since 1.1.2:
#547

I believe the logic can be improved by not running dbt deps if there is no packages.yml or it is empty. Any contributions in this sense would be welcome.

@tatiana tatiana added the parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing label Nov 8, 2023
Copy link

dosubot bot commented Feb 7, 2024

Hi, @nicohein,

I'm helping the Cosmos team manage their backlog and am marking this issue as stale. From what I understand, you raised an issue regarding a change in version 1.2.0 that causes an error when using LoadMode.DBT_LS, requiring render_config.dbt_deps to be set to True. There was a suggestion to improve the logic to avoid running dbt deps if there are no packages.yml or if it is empty, and a reproducible example of the issue was requested.

Could you please confirm if this issue is still relevant to the latest version of the Cosmos repository? If it is, please let the Cosmos team know by commenting on the issue. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days.

Thank you for your understanding and cooperation.

@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Feb 7, 2024
@nicohein
Copy link
Author

nicohein commented Feb 7, 2024

Hi @tatiana,
I have not invested more tim ein this and have solved the issues with a different setup altogether. As I will unfortunately not have much more time to contribute here, it is fine with me to close this.
Best
Nico

@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Feb 7, 2024
@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2024
Copy link

dosubot bot commented Feb 7, 2024

🤖

Hi @nicohein,

Thank you for your response and understanding. We appreciate your cooperation. We'll go ahead and close this issue now.

Best regards,
The Cosmos Team


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting_response parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing
Projects
None yet
Development

No branches or pull requests

2 participants