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

Store compiled_sql even when task fails (fixes issue #369) #671

Merged

Conversation

agreenburg
Copy link
Contributor

@agreenburg agreenburg commented Nov 14, 2023

Description

Updated DbtLocalBaseOperator code to store compiled_sql prior to exception handling so that when a task fails the compiled_sql can still be reviewed.

In the process found and fixed a related bug where compiled_sql was being dropped on some operations due to the way that the full_refresh field was being added to the template_fields.

Related Issue(s)

closes #369

Fixes bug introduced in #623 where compiled_sql was being lost in DbtSeedLocalOperator and DbtRunLocalOperator

Breaking Change?

Not a breaking change

Checklist

  • I have made corresponding changes to the documentation (if required)
  • I have added tests that prove my fix is effective or that my feature works

@agreenburg agreenburg requested a review from a team as a code owner November 14, 2023 16:08
@agreenburg agreenburg requested a review from a team November 14, 2023 16:08
Copy link

netlify bot commented Nov 14, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit d2d0d47
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/65539c69ae6b260008184090

@dosubot dosubot bot added area:execution Related to the execution environment/mode, like Docker, Kubernetes, Local, VirtualEnv, etc dbt:compile Primarily related to dbt compile command or functionality execution:local Related to Local execution environment labels Nov 14, 2023
@agreenburg
Copy link
Contributor Author

I wasn't sure if additional unit tests are needed for this. I can add them if so.

Copy link

codecov bot commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0f16d15) 92.75% compared to head (d2d0d47) 92.75%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #671   +/-   ##
=======================================
  Coverage   92.75%   92.75%           
=======================================
  Files          54       54           
  Lines        2235     2235           
=======================================
  Hits         2073     2073           
  Misses        162      162           

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

Copy link
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks for the quick fix, @agreenburg !

@tatiana tatiana merged commit ee91ece into astronomer:main Nov 14, 2023
42 checks passed
tatiana pushed a commit that referenced this pull request Nov 15, 2023
Update `DbtLocalBaseOperator` code to store `compiled_sql` prior to
exception handling so that when a task fails, the `compiled_sql` can still
be reviewed.

In the process found and fixed a related bug where `compiled_sql` was
being dropped on some operations due to the way that the `full_refresh`
field was being added to the `template_fields`.

Closes #369

Fixes bug introduced in
#623 where
compiled_sql was being lost in `DbtSeedLocalOperator` and
`DbtRunLocalOperator`

Co-authored-by: Andrew Greenburg <[email protected]>
(cherry picked from commit ee91ece)
tatiana added a commit that referenced this pull request Nov 15, 2023
Bug fixes

* Store `compiled_sql` even when task fails by @agreenburg in #671
* Refactor `LoadMethod.LOCAL` to use symlinks instead of copying directory by @jbandoro in #660
* Fix 'Unable to find the dbt executable: dbt' error by @tatiana in #666
* Fix installing deps when using `profile_mapping` & `ExecutionMode.LOCAL` by @joppevos in #659

Others

* Docs fix: add execution config to MWAA code example by @ugmuka in #674
@tatiana tatiana mentioned this pull request Nov 15, 2023
tatiana added a commit that referenced this pull request Nov 15, 2023
Bug fixes

* Store `compiled_sql` even when task fails by @agreenburg in #671
* Refactor `LoadMethod.LOCAL` to use symlinks instead of copying directory by @jbandoro in #660
* Fix 'Unable to find the dbt executable: dbt' error by @tatiana in #666
* Fix installing deps when using `profile_mapping` & `ExecutionMode.LOCAL` by @joppevos in #659

Others

* Docs fix: add execution config to MWAA code example by @ugmuka in #674

(cherry picked from commit aa9b7bb)
@tatiana tatiana mentioned this pull request Nov 15, 2023
tatiana pushed a commit that referenced this pull request Nov 15, 2023
Update `DbtLocalBaseOperator` code to store `compiled_sql` prior to
exception handling so that when a task fails, the `compiled_sql` can still
be reviewed.

In the process found and fixed a related bug where `compiled_sql` was
being dropped on some operations due to the way that the `full_refresh`
field was being added to the `template_fields`.

Closes #369

Fixes bug introduced in
#623 where
compiled_sql was being lost in `DbtSeedLocalOperator` and
`DbtRunLocalOperator`

Co-authored-by: Andrew Greenburg <[email protected]>
(cherry picked from commit ee91ece)
tatiana added a commit that referenced this pull request Nov 15, 2023
Bug fixes

* Store `compiled_sql` even when task fails by @agreenburg in #671
* Refactor `LoadMethod.LOCAL` to use symlinks instead of copying directory by @jbandoro in #660
* Fix 'Unable to find the dbt executable: dbt' error by @tatiana in #666
* Fix installing deps when using `profile_mapping` & `ExecutionMode.LOCAL` by @joppevos in #659

Others

* Docs fix: add execution config to MWAA code example by @ugmuka in #674
@tatiana tatiana added this to the 1.2.4 milestone Nov 16, 2023
@tatiana
Copy link
Collaborator

tatiana commented Nov 16, 2023

Thanks for the contribution, @agreenburg , we released it as part of Cosmos 1.2.4 yesterday:
https:/astronomer/astronomer-cosmos/releases/tag/astronomer-cosmos-v1.2.4

tatiana added a commit that referenced this pull request Nov 16, 2023
**Bug fixes**

* Store `compiled_sql` even when task fails by @agreenburg in #671
* Refactor `LoadMethod.LOCAL` to use symlinks instead of copying
directory by @jbandoro in #660
* Fix 'Unable to find the dbt executable: dbt' error by @tatiana in #666
* Fix installing deps when using `profile_mapping` &
`ExecutionMode.LOCAL` by @joppevos in #659

**Others**

* Docs fix: add execution config to MWAA code example by @ugmuka in #674

(cherry picked from commit aa9b7bb)
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
…stronomer#671)

Update `DbtLocalBaseOperator` code to store `compiled_sql` prior to
exception handling so that when a task fails, the `compiled_sql` can still
be reviewed.

In the process found and fixed a related bug where `compiled_sql` was
being dropped on some operations due to the way that the `full_refresh`
field was being added to the `template_fields`.

Closes astronomer#369

Fixes bug introduced in
astronomer#623 where
compiled_sql was being lost in `DbtSeedLocalOperator` and
`DbtRunLocalOperator`

Co-authored-by: Andrew Greenburg <[email protected]>
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this pull request Jul 14, 2024
**Bug fixes**

* Store `compiled_sql` even when task fails by @agreenburg in astronomer#671
* Refactor `LoadMethod.LOCAL` to use symlinks instead of copying
directory by @jbandoro in astronomer#660
* Fix 'Unable to find the dbt executable: dbt' error by @tatiana in astronomer#666
* Fix installing deps when using `profile_mapping` &
`ExecutionMode.LOCAL` by @joppevos in astronomer#659

**Others**

* Docs fix: add execution config to MWAA code example by @ugmuka in astronomer#674

(cherry picked from commit aa9b7bb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:execution Related to the execution environment/mode, like Docker, Kubernetes, Local, VirtualEnv, etc dbt:compile Primarily related to dbt compile command or functionality execution:local Related to Local execution environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rendered SQL is not available for failed runs.
2 participants