Skip to content

Fix caching unique id error#1295

Closed
tatiana wants to merge 4 commits into
mainfrom
fix-caching-unique-id-error
Closed

Fix caching unique id error#1295
tatiana wants to merge 4 commits into
mainfrom
fix-caching-unique-id-error

Conversation

@tatiana
Copy link
Copy Markdown
Collaborator

@tatiana tatiana commented Oct 31, 2024

Workaround for issues faced by customers in the production.

An Astronomer customer using Cosmos 1.5.0 with caching enabled in production faces failures in their DAGs once they change those DAGs or dbt projects using astro deply --dags. DAGs that were not changed continue to work.

They are using cache based on Airflow Variables.

The error they are facing is:

    File /usr/local/lib/python3.11/site-packages/cosmos/dbt/graph.py, line 135, in parse_dbt_ls_output
        unique_id=node_dict[unique_id]
    KeyError: 'unique_id'

This change aims to add logs and skip nodes that may not contain the necessary fields. This will allow us to identify the root cause.

pankajkoti and others added 4 commits June 27, 2024 20:06
An Astronomer customer that is using Cosmos 1.5 with caching enabled in production is facing failures in their
DAGs once they change those DAGs or dbt projects.

They are using cache based on Airflow Variables.

The error they are facing is:

File /usr/local/lib/python3.11/site-packages/cosmos/dbt/graph.py, line 135, in parse_dbt_ls_output
    unique_id=node_dict[unique_id]
KeyError: 'unique_id'

This change aims to add additional logs and skip nodes that may not contain necessary fields. This will allow us to identify the root cause
@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 31, 2024

Deploy Preview for sunny-pastelito-5ecb04 ready!

Name Link
🔨 Latest commit 695e791
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/67236d971efaa2000820fc55
😎 Deploy Preview https://deploy-preview-1295--sunny-pastelito-5ecb04.netlify.app
📱 Preview on mobile
Toggle QR Code...

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
Copy link
Copy Markdown
Collaborator Author

tatiana commented Oct 31, 2024

We identified what was the problem. There was a macro that would print JSONs in dbt ls that were not meant to be handled as dbt nodes.

This was the dbt ls output, before printing the actual nodes:

11:20:43  Running with dbt=1.7.6
11:20:45  Registered adapter: bigquery=1.7.2
11:20:45  Unable to do partial parsing because saved manifest not found. Starting full parse.
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
/***************************/ 
Values returned by mac_get_values: 
{}
/***************************/
11:20:55  [WARNING]: Configuration paths exist in your dbt_project.yml file which do not apply to any resources.
11:20:55  Found 85 models, 28 tests, 1 seed, 21 sources, 0 exposures, 0 metrics, 911 macros, 0 groups, 0 semantic models

We have two solutions for the customer who faced this issue:

  1. Modify the macro print statement to be one-line (e.g. 'Values returned by mac_get_values: {}')
  2. To use cosmos 1.5.0rc2, which is more resilient to this use-case, and simply skips lines that do not have the expected dbt node keys

In the meantime, I'm going to close this PR and create a new PR against Cosmos main branch, and add a test-case for this.

@tatiana tatiana closed this Oct 31, 2024
@tatiana tatiana added this to the Cosmos 1.8.0 milestone Oct 31, 2024
tatiana added a commit that referenced this pull request Oct 31, 2024
…1296)

This change makes Cosmos more resilient, allowing it to be used even
when JSONs do not represent dbt nodes in the `dbt ls` output.

**Context**

An Astronomer customer [raised a P1
incident](https://astronomer.zendesk.com/agent/tickets/67681),
mentioning they could no longer run their Cosmos-powered DAGs.

They were using Cosmos 1.5.0, and the issue was observed whenever DAGs
were deployed using `Astro deploy --dags`, even if they only had
whitespace as a difference.

The DAGs could no longer be parsed, raising an exception similar to:

```
    File /usr/local/lib/python3.11/site-packages/cosmos/dbt/graph.py, line 135, in parse_dbt_ls_output
        unique_id=node_dict[unique_id]
    KeyError: 'unique_id'
```

**Explanation**

The customer recently changed their dbt project, adding print debug
statements to one of their dbt macros.

This caused the dbt ls output to contain lines that were valid JSON but
were not valid dbt nodes, as observed in:
```
11:20:43  Running with dbt=1.7.6
11:20:45  Registered adapter: bigquery=1.7.2
11:20:45  Unable to do partial parsing because saved manifest not found. Starting full parse.
/***************************/
Values returned by mac_get_values:
{}
/***************************/
{"name": "some_model", "resource_type": "model", "package_name": "some_package", "original_file_path": "models/some_model.sql", "unique_id": "model.some_package.some_model", "alias": "some_model_some_package_1_8_0", "config": {"enabled": true, "alias": "some_model_some_package-1.8.0", "schema": "some_schema", "database": null, "tags": [], "meta": {}, "group": null, "materialized": "view", "incremental_strategy": null, "persist_docs": {}, "post-hook": [], "pre-hook": [], "quoting": {}, "column_types": {}, "full_refresh": null, "unique_key": null, "on_schema_change": "ignore", "on_configuration_change": "apply", "grants": {}, "packages": [], "docs": {"show": true, "node_color": null}, "contract": {"enforced": false, "alias_types": true}, "access": "protected"}, "tags": [], "depends_on": {"macros": [], "nodes": ["source.some_source"]}}"""
```

Cosmos didn't consider this use case. It assumed if a line was a JSON,
it should be a dbt node:

https://github.com/astronomer/astronomer-cosmos/blob/42a397fb40ff537c74bb6f596b4936815b14abbb/cosmos/dbt/graph.py#L161-L185

**Workaround**

If customers updated the macro to print the information in a single
line, they'd no longer observe the issue:
```
Values returned by mac_get_values: {}
```

We also released
[1.5.0rc2](https://github.com/astronomer/astronomer-cosmos/releases/tag/astronomer-cosmos-v1.5.0rc2)
with the change #1295, similar to the one introduced by this PR.

**Fix**

This change makes Cosmos more resilient to scenarios where `dbt ls` may
output JSON lines that are not valid dbt nodes. It also logs those lines
to help troubleshoot. We added a unit test to make sure we continue
supporting this use-case.
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

Successfully merging this pull request may close these issues.

2 participants