Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-21 Thread via GitHub
potiuk merged PR #35653: URL: https://github.com/apache/airflow/pull/35653 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.a

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-21 Thread via GitHub
jscheffl commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1957365783 So, GREEN! Now just another second reviewer needed here... then for me this is looking good. -- This is an automated message from the Apache Git Service. To respond to the message, p

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-21 Thread via GitHub
potiuk commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1956447765 (In this case it would have saved you 2 hours) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to g

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-21 Thread via GitHub
potiuk commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1956446729 I clicked "Rebase" button - but it's always faster for you to do rather than wait for someone (in the future) @hterik -- This is an automated message from the Apache Git Service.

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-21 Thread via GitHub
potiuk commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1956445309 > Thanks @jscheffl , can you please retrigger the failing CI actions? I've run all the unit tests locally now and the CI failures are passing locally. No you need to rebase for tha

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-21 Thread via GitHub
hterik commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1956220869 Thanks @jscheffl , can you please retrigger the failing CI actions? I've run all the unit tests locally now and the CI failures are passing locally. -- This is an automated message fro

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-20 Thread via GitHub
jscheffl commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1954980699 @hterik before I do a full test, do you have any recommondation how to test? Did you use any example DAG and timeout? -- This is an automated message from the Apache Git Service. To

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-20 Thread via GitHub
potiuk commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1954351092 > Latest CI failures look unrelated to change, can someone please help rerun it. The main i broken, and it's being fixed in #37559 > In the meantime, I'm really struggling

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2024-02-20 Thread via GitHub
hterik commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1954140677 Latest CI failures look unrelated to change, can someone please help rerun it. (`Airflow FS S3 protocol requires the s3fs library, but it is not installed as it requiresaiobotocor

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-12-31 Thread via GitHub
jscheffl commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1873017894 I was reading through the discussion and found this PR now being a bit stale. I'd make a full review if nobody objects. Before merge though I would kindly request a re-base and

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-12-30 Thread via GitHub
github-actions[bot] commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1872632280 This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for you

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
potiuk commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1812349726 > An in general need to check other Airflow codebase (core), that we do not miss to catch `AirflowTaskTimeout` where it required, because right now we could catch as `AirflowException` i

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
hterik commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1812328332 > An in general need to check other Airflow codebase (core), that we do not miss to catch `AirflowTaskTimeout` where it required, because right now we could catch as `AirflowException` i

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
Taragolis commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1812312826 For example `StandardTaskRunner` https://github.com/apache/airflow/blob/1e24a3cf96c2b14b69532c604bdedb8f9ea720de/airflow/task/task_runner/standard_task_runner.py#L104-L133 --

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
Taragolis commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1812279786 An in general need to check other Airflow codebase (core), that we do not miss to catch `AirflowTaskTimeout` where it required, because right now we could catch as `AirflowException`

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
potiuk commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1812193221 > Hmm, SIGALARM wouldn't be injected in the middle of C-extension execution but it would stay pending until the extension returns to Python execution at least right? So it won't be total

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
hterik commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1812174736 Hmm, SIGALARM wouldn't be injected in the middle of C-extension execution but it would stay pending until the extension returns to Python execution at least right? So it won't be totally

Re: [PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
potiuk commented on PR #35653: URL: https://github.com/apache/airflow/pull/35653#issuecomment-1812164143 > Code that normally catches Exception should not implicitly ignore interrupts from AirflowTaskTimout. > > Fixes #35644 #35474 I do not think #35474 is fixed by it. We STILL

[PR] Change AirflowTaskTimeout to inherit BaseException [airflow]

2023-11-15 Thread via GitHub
hterik opened a new pull request, #35653: URL: https://github.com/apache/airflow/pull/35653 Code that normally catches Exception should not implicitly ignore interrupts from AirflowTaskTimout. Fixes #35644 #35474 -- This is an automated message from the Apache Git Service.