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

Feat/1749 abort load package and raise exception on terminal errors in jobs #1781

Conversation

willi-mueller
Copy link
Collaborator

@willi-mueller willi-mueller commented Sep 3, 2024

@willi-mueller willi-mueller linked an issue Sep 3, 2024 that may be closed by this pull request
@willi-mueller willi-mueller marked this pull request as ready for review September 3, 2024 11:57
Copy link

netlify bot commented Sep 3, 2024

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit 05e324c
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/66e04ab80a6d0f00086ed576

Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

How we need to test it

  1. there are tests in dummy client that are testing abort feature, there just remove the flag. it is by default on
  2. make the other tests to pass by disabling the flag

look for raise_on_failed_jobs in the whole code
we use it to raise on load info, those are not needed anymore and can be removed (many of those)

I see that we set this flag in airflow helper. we should change the default there and also we never set it to false

there are other cases. you'll find them for sure

docs/website/docs/running-in-production/running.md Outdated Show resolved Hide resolved
@willi-mueller willi-mueller force-pushed the feat/1749-abort-load-package-and-raise-exception-on-terminal-errors-in-jobs branch from b305d9b to 932c160 Compare September 9, 2024 07:53
@willi-mueller willi-mueller force-pushed the feat/1749-abort-load-package-and-raise-exception-on-terminal-errors-in-jobs branch from b891bc1 to 2606217 Compare September 9, 2024 08:37
@willi-mueller willi-mueller marked this pull request as draft September 9, 2024 10:28
@willi-mueller willi-mueller force-pushed the feat/1749-abort-load-package-and-raise-exception-on-terminal-errors-in-jobs branch from e851b84 to 864f642 Compare September 9, 2024 15:31
@willi-mueller willi-mueller self-assigned this Sep 9, 2024
@willi-mueller willi-mueller marked this pull request as ready for review September 9, 2024 16:30
rudolfix
rudolfix previously approved these changes Sep 9, 2024
Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

LGTM!

dlt/helpers/airflow_helper.py Outdated Show resolved Hide resolved
tests/pipeline/test_pipeline.py Outdated Show resolved Hide resolved
tests/pipeline/test_pipeline.py Outdated Show resolved Hide resolved
@rudolfix rudolfix force-pushed the feat/1749-abort-load-package-and-raise-exception-on-terminal-errors-in-jobs branch from 83b9b15 to aa66a49 Compare September 10, 2024 08:50
@rudolfix rudolfix force-pushed the feat/1749-abort-load-package-and-raise-exception-on-terminal-errors-in-jobs branch from aa66a49 to 74697d8 Compare September 10, 2024 10:33
@rudolfix rudolfix merged commit 79c70c9 into devel Sep 10, 2024
25 of 26 checks passed
@rudolfix rudolfix deleted the feat/1749-abort-load-package-and-raise-exception-on-terminal-errors-in-jobs branch September 10, 2024 13:34
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.

abort load package and raise exception on terminal errors in jobs
2 participants