Kubernetes on warning callback #673

david-mag · 2023-11-14T22:20:45Z

Description

In order to make on_warning_callback work with pod operators, we need to read the logs of the dbt test runs. This is done by making sure the pod is kept alive, and on_success_callback the log is read and anlayzed for warnings.
Afterwards the pod is cleaned up, based on the original settings from the user.

If on_warning_callback is not set, everything stays the way it always was.

I've tested the changes in my dev environment, but haven't added any new tests, since this would be something (I think) we would need to add integration tests for.

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

netlify · 2023-11-14T22:20:51Z

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name	Link
🔨 Latest commit	`af2314e`
🔍 Latest deploy log	https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/655d6560f3f37d00087ae378

codecov · 2023-11-15T12:39:13Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a83911f) 92.88% compared to head (f91ac6c) 92.93%.

❗ Current head f91ac6c differs from pull request most recent head af2314e. Consider uploading reports for the commit af2314e to get more accurate results

Files	Patch %	Lines
cosmos/operators/kubernetes.py	95.74%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #673      +/-   ##
==========================================
+ Coverage   92.88%   92.93%   +0.04%     
==========================================
  Files          55       55              
  Lines        2248     2292      +44     
==========================================
+ Hits         2088     2130      +42     
- Misses        160      162       +2

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

tatiana

Hey, @david-mag, this looks very good - it's great to have feature parity to use on_warning_callback on Kubernetes.

I'm apprehensive about the need for tests. An integration test would be ideal - and we could do that once #535 is completed.

In the meantime, how would you feel about adding some unit tests to the new methods and the class constructor?

…/astronomer-cosmos into kubernetes_on_warning_callback

david-mag · 2023-11-15T22:05:28Z

I've added some unit-tests that (I hope) will cover my additions to the DbtTestKubernetesOperator. I had to, however, add a condition to skip the test for airflow version 2.3, because there the OnFinishAction from airflow.providers.cncf.kubernetes.utils.pod_manager doesn't exist yet.

So using on_warning_callback will only be possible from airflow version 2.4 and up.

tatiana

@david-mag thanks for the contribution!

This will improve K8s users experience!

There will be scenarios when OnFinishAction won't be defined. If the user sets on_warning_callback and uses ExecutionMode.KUBERNETES, DbtTestKubernetesOperator will only work if users use apache-airflow-providers-cncf-kubernetes >= 7.4.0. I believe this is a fine compromise, considering that before this PR on_warning_callback wasn't working for any version of the provider when users used ExecutionMode.KUBERNETES.

tatiana · 2023-11-23T16:52:47Z

@david-mag This was released in 1.2.5 🎉

@binhnq94

Bug fixes * Fix running models that use alias while supporting dbt versions by @binhnq94 in astronomer#662 * Make profiles_yml_path optional for ExecutionMode.DOCKER and KUBERNETES by @MrBones757 in astronomer#681 * Prevent overriding dbt profile fields with profile args of type or method by @jbandoro in astronomer#702 * Fix LoadMode.DBT_LS fail when dbt outputs WarnErrorOptions by @adammarples in astronomer#692 * Add support for env vars in RenderConfig for dbt ls parsing by @jbandoro in astronomer#690 * Add support for Kubernetes on_warning_callback by @david-mag in astronomer#673 * Fix ExecutionConfig.dbt_executable_path to use ``default_factory`` by @jbandoro in astronomer#678 Others * Docs fix: example DAG in the README and docs/index by @tatiana in astronomer#705 * Docs improvement: highlight DAG examples in README by @iancmoritz and @jlaneve in astronomer#695 (cherry picked from commit 2878d6a)

To make `on_warning_callback` work with pod operators, we need to read the logs of the dbt test runs. This is done by ensuring the pod is kept alive, and `on_success_callback` the log is read and analysed for warnings. Afterwards, the pod is cleaned up based on the original settings from the user. If `on_warning_callback` is not set, everything stays the way it always was. This feature only work with `apache-airflow-providers-cncf-kubernetes >= 7.4.0`.

@binhnq94

Bug fixes * Fix running models that use alias while supporting dbt versions by @binhnq94 in astronomer#662 * Make profiles_yml_path optional for ExecutionMode.DOCKER and KUBERNETES by @MrBones757 in astronomer#681 * Prevent overriding dbt profile fields with profile args of type or method by @jbandoro in astronomer#702 * Fix LoadMode.DBT_LS fail when dbt outputs WarnErrorOptions by @adammarples in astronomer#692 * Add support for env vars in RenderConfig for dbt ls parsing by @jbandoro in astronomer#690 * Add support for Kubernetes on_warning_callback by @david-mag in astronomer#673 * Fix ExecutionConfig.dbt_executable_path to use ``default_factory`` by @jbandoro in astronomer#678 Others * Docs fix: example DAG in the README and docs/index by @tatiana in astronomer#705 * Docs improvement: highlight DAG examples in README by @iancmoritz and @jlaneve in astronomer#695 (cherry picked from commit 2878d6a)

david-mag added 2 commits November 14, 2023 22:59

add on_warning_callback functionality

4422481

fix

cf0b5a9

david-mag requested a review from a team as a code owner November 14, 2023 22:20

david-mag requested a review from a team November 14, 2023 22:20

david-mag had a problem deploying to external November 14, 2023 22:20 — with GitHub Actions Error

dosubot bot added area:execution Related to the execution environment/mode, like Docker, Kubernetes, Local, VirtualEnv, etc dbt:test Primarily related to dbt test command or functionality execution:kubernetes Related to Kubernetes execution environment labels Nov 14, 2023

bugfixes

76c2d65

david-mag temporarily deployed to external November 14, 2023 22:34 — with GitHub Actions Inactive

tatiana reviewed Nov 15, 2023

View reviewed changes

add dbttestkubernetesoperator tests

0688dca

david-mag had a problem deploying to external November 15, 2023 21:27 — with GitHub Actions Error

skip tests for some versions of airflow

3bb70cc

david-mag had a problem deploying to external November 15, 2023 21:55 — with GitHub Actions Error

pre-commit-ci bot and others added 3 commits November 15, 2023 21:56

🎨 [pre-commit.ci] Auto format from pre-commit.com hooks

c4d3468

formatting fixes

eec0221

Merge branch 'kubernetes_on_warning_callback' of github.com:david-mag…

5e79dd8

…/astronomer-cosmos into kubernetes_on_warning_callback

david-mag temporarily deployed to external November 15, 2023 21:56 — with GitHub Actions Inactive

tatiana added this to the 1.2.5 milestone Nov 16, 2023

Merge branch 'main' into kubernetes_on_warning_callback

6a153dc

tatiana temporarily deployed to external November 16, 2023 15:06 — with GitHub Actions Inactive

Merge branch 'main' into kubernetes_on_warning_callback

3c66f5c

tatiana had a problem deploying to external November 17, 2023 13:30 — with GitHub Actions Error

Merge branch 'main' into kubernetes_on_warning_callback

f91ac6c

tatiana temporarily deployed to external November 18, 2023 08:05 — with GitHub Actions Inactive

Merge branch 'main' into kubernetes_on_warning_callback

af2314e

tatiana temporarily deployed to external November 22, 2023 02:20 — with GitHub Actions Inactive

tatiana approved these changes Nov 22, 2023

View reviewed changes

tatiana merged commit 0b538a5 into astronomer:main Nov 22, 2023
39 checks passed

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Nov 22, 2023

This was referenced Nov 23, 2023

DO NOT MERGE - Release 1.2.5 #708

Closed

Release 1.2.5 #709

Merged

Release 1.2.5 #710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubernetes on warning callback #673

Kubernetes on warning callback #673

david-mag commented Nov 14, 2023 •

edited

Loading

netlify bot commented Nov 14, 2023 •

edited

Loading

codecov bot commented Nov 15, 2023 •

edited

Loading

tatiana left a comment •

edited

Loading

david-mag commented Nov 15, 2023 •

edited

Loading

tatiana left a comment

tatiana commented Nov 23, 2023

Kubernetes on warning callback #673

Kubernetes on warning callback #673

Conversation

david-mag commented Nov 14, 2023 • edited Loading

Description

Checklist

netlify bot commented Nov 14, 2023 • edited Loading

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

codecov bot commented Nov 15, 2023 • edited Loading

Codecov Report

tatiana left a comment • edited Loading

Choose a reason for hiding this comment

david-mag commented Nov 15, 2023 • edited Loading

tatiana left a comment

Choose a reason for hiding this comment

tatiana commented Nov 23, 2023

david-mag commented Nov 14, 2023 •

edited

Loading

netlify bot commented Nov 14, 2023 •

edited

Loading

codecov bot commented Nov 15, 2023 •

edited

Loading

tatiana left a comment •

edited

Loading

david-mag commented Nov 15, 2023 •

edited

Loading