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

dbt ls fails on "Error" string in stdout #642

Closed
adammarples opened this issue Nov 2, 2023 · 10 comments
Closed

dbt ls fails on "Error" string in stdout #642

adammarples opened this issue Nov 2, 2023 · 10 comments
Labels
area:parsing Related to parsing DAG/DBT improvement, issues, or fixes bug Something isn't working priority:high High priority issues are blocking or critical issues without a workaround and large impact
Milestone

Comments

@adammarples
Copy link
Contributor

adammarples commented Nov 2, 2023

This error check https://github.com/astronomer/astronomer-cosmos/blob/635fb7badfd1fea560923d3f959ab7d021dba83d/cosmos/dbt/graph.py#L253C1-L255C111

if returncode or "Error" in stdout:
    details = stderr or stdout
    raise CosmosLoadDbtException(f"Unable to run dbt ls command due to the error:\n{details}")

Causes a failure when stdout includes (in my example)

Broken DAG: [/usr/local/airflow/dags/playground.py] Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/cosmos/dbt/graph.py", line 132, in load
    load_method[method]()
  File "/usr/local/lib/python3.11/site-packages/cosmos/dbt/graph.py", line 255, in load_via_dbt_ls
    raise CosmosLoadDbtException(f"Unable to run dbt ls command due to the error:\n{details}")
cosmos.dbt.graph.CosmosLoadDbtException: Unable to run dbt ls command due to the error:
�[0m12:48:41  Sending event: {'category': 'dbt', 'action': 'invocation', 'label': 'start', 'context': [<snowplow_tracker.self_describing_json.SelfDescribingJson object at 0xffff85fc0110>, <snowplow_tracker.self_describing_json.SelfDescribingJson object at 0xffff860efe50>, <snowplow_tracker.self_describing_json.SelfDescribingJson object at 0xffff85dfbc90>]}
�[0m12:48:41  Running with dbt=1.6.7
�[0m12:48:41  running dbt with arguments {'printer_width': '80', 'indirect_selection': 'eager', 'log_cache_events': 'False', 'write_json': 'True', 'partial_parse': 'True', 'cache_selected_only': 'False', 'warn_error': 'None', 'debug': 'True', 'profiles_dir': '/tmp/tmp1v3xloax', 'log_path': '/tmp/tmp6rmp2huk/logs', 'fail_fast': 'False', 'version_check': 'True', 'use_colors': 'True', 'use_experimental_parser': 'False', 'no_print': 'None', 'quiet': 'False', 'warn_error_options': 'WarnErrorOptions(include=[], exclude=[])', 'static_parser': 'True', 'introspect': 'True', 'invocation_command': 'dbt ls --output json --exclude tag:qc_test --select stage --project-dir /tmp/tmp6rmp2huk --profiles-dir /tmp/tmp1v3xloax --profile vulcan_profile --target pr', 'target_path': '/tmp/tmp6rmp2huk/target', 'log_format': 'default', 'send_anonymous_usage_stats': 'True'}

Specifically "Error" is being found in the last line here

'warn_error_options': 'WarnErrorOptions(include=[], exclude=[])'

This check is overly broad and doesn' allow people to have ie. directories, model names, tags, schemas etc. with "Error" in them.

@tatiana
Copy link
Collaborator

tatiana commented Nov 2, 2023

Hi @adammarples, I'm sorry you're facing this issue. I fully agree we need to improve this.

This was introduced because the dbt may times raise errors, but without using the adequate OS process return code.

Would you like to contribute to improving this part of Cosmos?

@tatiana tatiana added bug Something isn't working priority:high High priority issues are blocking or critical issues without a workaround and large impact labels Nov 2, 2023
@tatiana tatiana added this to the 1.3.0 milestone Nov 2, 2023
@adammarples
Copy link
Contributor Author

I would love to help, it sounds like an issue with dbt-core primarily though. Do you have any info about when these errors occur without adequate returncodes or why you started searching for the "Error" string in the first place? Thanks

@tatiana
Copy link
Collaborator

tatiana commented Nov 2, 2023

@adammarples, I agree that, ideally, this would be resolved in dbt itself.

The dbt documentation states the exit code would be 0 only if the dbt invocation is completed without error:
https://docs.getdbt.com/reference/exit-codes

However, it seems there was a decision to have dbt ls always return 0, which is not compatible with the documentation:
dbt-labs/dbt-core#2892

I remember two scenarios when we needed to capture errors raised while running dbt ls, which lead to the current implementation:

  1. When there were missing adapters: Run dbt deps when using load_mode=LoadMode.DBT_LS #445
  2. If using a user-defined profiles.yml that contained references to unset environment variables

@jlaneve
Copy link
Collaborator

jlaneve commented Nov 2, 2023

@jtcohen6 curious if you have any thoughts here - running dbt ls can actually trigger an error but the exit code is always 0. I'd be happy to look into contributing back to the dbt core codebase if you agree the dbt ls command should actually return non-zero codes

@adammarples
Copy link
Contributor Author

As a workaround, I wonder if something like

if returncode or "Error" in stdout.replace("WarnErrorOptions", ""):

Would be acceptable because I suspect that WarnErrorOptions is going to turn up a lot

@tatiana tatiana added the area:parsing Related to parsing DAG/DBT improvement, issues, or fixes label Nov 8, 2023
@tatiana
Copy link
Collaborator

tatiana commented Nov 17, 2023

@adammarples yes, I believe this would be a valid workaround - would you like to make this contribution to Cosmos?

@adammarples
Copy link
Contributor Author

@adammarples yes, I believe this would be a valid workaround - would you like to make this contribution to Cosmos?

would be happy to

@adammarples
Copy link
Contributor Author

@tatiana #692

jbandoro pushed a commit to jbandoro/astronomer-cosmos that referenced this issue Nov 22, 2023
Ignore `WarnErrorOptions` string in stdout from dbt-core, do not treat it
as a failure

Closes: astronomer#642
@tatiana
Copy link
Collaborator

tatiana commented Dec 7, 2023

@adammarples Since we've released the workaround in 1.2.5 and don't have a better solution now, I'm closing this ticket. Please, feel free to reopen or open a new one if you have any suggestions of how we could better handle this.

@tatiana tatiana closed this as completed Dec 7, 2023
@adammarples
Copy link
Contributor Author

@tatiana actually I think I have a very simple workaround for this if it isn't fixed in dbt-core. We can force specific warnings to become errors for the dbt ls command and force them to return exit code 2.

For example, to raise an error for the issue that dbt-core were initially trying to fix, no nodes selected on dbt ls, we could catch that warning and turn it into an error when running ls.

dbt --warn-error-options '{"include": ["NoNodesForSelectionCriteria"]}' ls 

For the issues you mentioned:

  1. When there were missing adapters: Run dbt deps when using load_mode=LoadMode.DBT_LS #445
  2. If using a user-defined profiles.yml that contained references to unset environment variables

I tested 2 and it actually already returns an error code 2. I'm not sure what 1 exactly is, but it seems to be the same env_vars issue?

arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this issue Jul 14, 2024
Ignore `WarnErrorOptions` string in stdout from dbt-core, do not treat it
as a failure

Closes: astronomer#642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:parsing Related to parsing DAG/DBT improvement, issues, or fixes bug Something isn't working priority:high High priority issues are blocking or critical issues without a workaround and large impact
Projects
None yet
Development

No branches or pull requests

3 participants