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

Analysis ResourceType #579

Closed
nicohein opened this issue Oct 8, 2023 · 7 comments
Closed

Analysis ResourceType #579

nicohein opened this issue Oct 8, 2023 · 7 comments

Comments

@nicohein
Copy link

nicohein commented Oct 8, 2023

When testing astronomer-cosmos with the manifest json parse method I am receiving the following error

ValueError: 'analysis' is not a valid DbtResourceType

However, according to the dbt documentation analysis is a valid resource type: https://docs.getdbt.com/reference/commands/list

I am not sure about the impact but would hope a simple addition to this Enum would solve the problem:

class DbtResourceType(Enum):

dbt version 1.5.7
astronomer-cosmos version 1.1.3

@erdos2n
Copy link
Contributor

erdos2n commented Oct 9, 2023

Hello @nicohein , I've seen similar errors thrown in tandem with a DagBag timeout error. Can you increase the DagBag import time AIRFLOW__CORE__DAGBAG_IMPORT_TIMEOUT and see if this issue still persists?

@tatiana
Copy link
Collaborator

tatiana commented Oct 9, 2023

@nicohein Cosmos 1.1.3 does not know how to handle nodes of type analysis.
How would you like those nodes to be displayed in Airlfow?
Once the PR #503 is merged, you should be able to define the desired behaviour.

@nicohein
Copy link
Author

@erdos2n yes, I increased the timeout to 300s, after seeing a DagBag timeout before. This error is independent. The Dag refill times for cosmos DAGs are really long though and varying a lot by Loadmode. DBT_LS with dbt_deps flag is by far the slowest on the setup I have.

@tatiana The issue is that as soon as there is an analysis node in the manifest, astronomer-cosmos is unable to parse the manifest. It can ignore and should ignore those nodes but it should still be able to parse the manifest and not throw an error. Changing the ValueError to a warning would equally work here and would probably be more future proof.

@tatiana
Copy link
Collaborator

tatiana commented Oct 10, 2023

Thanks, @nicohein! #503 should not fail for any new resource type - it would be great if you could validate it once we release this feature

@tatiana
Copy link
Collaborator

tatiana commented Oct 13, 2023

Hi @nicohein, we just released Cosmos 1.2:

Could you confirm if the issue is still happening with the new version?

@nicohein
Copy link
Author

Hi @tatiana ,

Initially, I got the below error:

  File "/opt/python3.8/lib/python3.8/site-packages/cosmos/airflow/task_group.py", line 26, in __init__
    DbtToAirflowConverter.__init__(self, *args, **specific_kwargs(**kwargs))
  File "/opt/python3.8/lib/python3.8/site-packages/cosmos/converter.py", line 109, in __init__
    dbt_root_path = project_config.dbt_project_path.parent
AttributeError: 'str' object has no attribute 'parent'

I can work around this problem by passing the dbt_project_path as a Path.

project_config = ProjectConfig(
    dbt_project_path=Path(DBT_HOME)
)

After this, I could then test for the ValueError: 'analysis' is not a valid DbtResourceType with LoadMode.DBT_MANIFEST and indeed the error is gone. Hence, we can close this issue.

@tatiana
Copy link
Collaborator

tatiana commented Oct 16, 2023

That's great, thank you very much, @nicohein !
We have a separate ticket related to the dbt_project_path supporting strings: #601

@tatiana tatiana closed this as completed Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants