Yeah, that should probably be the fix. The only question is if there are
other places that try to catch that with AirflowException (I don't think
so, it should only be in that spot linked). BTW, I think the reason others
don't see this issue is because this isn't a problem with cgroups enabled.

On Thu, Oct 3, 2019 at 8:19 AM Shaw, Damian P. <
damian.sha...@credit-suisse.com> wrote:

> Hi Alex,
>
> Maybe I'm stating the obvious but if any executors catches Airflow
> exceptions to do cleanup before exiting they can still do that with
> SystemExit. E.g.:
>
> class AirflowSystemExit(SystemExit):
>     pass
>
> try:
>     raise AirflowSystemExit
> except AirflowSystemExit:
>     print('cleanup for airflow throwing system exit')
> finally:
>     print('cleanup for any type of exit')
>
> In fact if people use either:
>
> try:
>     ...
> except:
>     ...
>
> Or
>
> try:
>     ...
> finally:
>     ...
>
> Their code will still run on this type of exit, but in the case of 1) this
> can at least be put down to poor python code and case 2) any code in a
> finally statement shouldn't be written so it can cause harm on exiting.
>
> Damian
>
>
> -----Original Message-----
> From: Alex Guziel [mailto:alex.guz...@airbnb.com.INVALID]
> Sent: Wednesday, October 02, 2019 4:03 PM
> To: dev@airflow.apache.org
> Subject: Re: How to manage Airflow SIGTERM excetion catches?
>
> Task_copy.on_kill() should probably be killing the underlying process, but
> I think it's fuzzy where the exception gets thrown. I think the intention
> is for the exception to get caught in that same block, so the cleanup can
> happen, but this is not the case since it is thrown in the main thread. I
> think the behavior depends a little bit on which executor you use. We
> probably need to think about the right behavior here that works for a
> variety of executors. SystemExit works for things that don't use
> subprocess, but I'm not sure what the behavior for cleanup is when running
> subprocess.
>
>
>
> ===============================================================================
>
> Please access the attached hyperlink for an important electronic
> communications disclaimer:
> http://www.credit-suisse.com/legal/en/disclaimer_email_ib.html
> ===============================================================================
>
>

Reply via email to