Hi all, Some people already gave some feedback on my approach but I would like to get some green/red light of the maintainers so I can start cleaning/changing the PR :)
Kind regards, Milan On Wed, Jan 17, 2018 at 5:09 PM, Milan van der Meer < [email protected]> wrote: > I have a first working version ready: https://github.com/ > apache/incubator-airflow/pull/2877 > > Only tested locally on my laptop with a BashOperator. > > On Tue, Jan 16, 2018 at 9:19 PM, Milan van der Meer <milan.vandermeer@ > realimpactanalytics.com> wrote: > >> I'll try to spend some time on this as well but it will take some time to >> get some understanding on the core code of Airflow. >> >> On Tue, Jan 16, 2018 at 9:12 PM, Joy Gao <[email protected]> wrote: >> >>> @Milan, I agree this bug needs to be fixed, and the the hack I provided >>> previously isn't complete as it doesn't work for any operator with >>> on_kill >>> method that requires transient task attributes (i.e. self.sp)... >>> Unfortunately I don't think the new UI will help rid of the issue (as the >>> bug is on the model layer, and running the cli command `clear` will >>> result >>> the same bug). I will try to set some time aside and look into this bug >>> again to see how to preserve the context, but currently don't have have a >>> good solution for it. >>> >>> >>> >>> On Mon, Jan 15, 2018 at 12:40 AM, EKC (Erik Cederstrand) < >>> [email protected]> >>> wrote: >>> >>> > I'm not sure if it's related, but there's an additional issue with >>> > attempting to kill processes, that doesn't always kill the process: >>> > https://issues.apache.org/jira/browse/AIRFLOW-949 >>> > >>> > >>> > Kind regards, >>> > >>> > Erik Cederstrand >>> > >>> > ________________________________ >>> > From: Milan van der Meer <[email protected]> >>> > Sent: Friday, January 12, 2018 9:23:24 PM >>> > To: [email protected] >>> > Subject: Re: Fix on_kill command for operators >>> > >>> > Currently the on_kill does not get triggered when you clear from the >>> UI. >>> > As you mention, adding the 'fix' mentioned in the first comment of the >>> > issue, does no fix the problem as it does not trigger on the right >>> > operators context. >>> > >>> > Im not sure what exact changes are planned for the next 1.10 release, >>> but >>> > if the whole UI change is planned, this could be a good opportunity to >>> also >>> > fix this bug. >>> > >>> > On Mon, Jan 8, 2018 at 2:55 PM, Ash Berlin-Taylor < >>> > [email protected]> wrote: >>> > >>> > > Without this change does on_kill ever get triggered? It seems like >>> this >>> > > change is desired behaviour. >>> > > >>> > > As per the first comment https://emea01.safelinks. >>> > protection.outlook.com/?url=https%3A%2F%2Fissues.apache. >>> > org%2F&data=01%7C01%7CEKC%40novozymes.com%7C4d2e0189d7e44025 >>> c48608d559fa >>> > 5913%7C43d5f49ee03a4d22a2285684196bb001%7C0&sdata=9%2BWOuDmUe9M1WK2% >>> > 2FqSsOtsaNrZnTtbvX%2Fq20dOBXVTA%3D&reserved=0 >>> > > jira/browse/AIRFLOW-1623?focusedCommentId=16171819& >>> > > page=com.atlassian.jira.plugin.system.issuetabpanels: >>> > > comment-tabpanel#comment-16171819 <https://emea01.safelinks. >>> > protection.outlook.com/?url=https%3A%2F%2Fissues.apache. >>> > org%2F&data=01%7C01%7CEKC%40novozymes.com%7C4d2e0189d7e44025 >>> c48608d559fa >>> > 5913%7C43d5f49ee03a4d22a2285684196bb001%7C0&sdata=9%2BWOuDmUe9M1WK2% >>> > 2FqSsOtsaNrZnTtbvX%2Fq20dOBXVTA%3D&reserved=0 >>> > > jira/browse/AIRFLOW-1623?focusedCommentId=16171819& >>> > > page=com.atlassian.jira.plugin.system.issuetabpanels: >>> > > comment-tabpanel#comment-16171819> I'm not sure this is the right >>> fix. >>> > It >>> > > also seems like this would end up running the on_kill in a different >>> > > process to the rest of the operator. >>> > > >>> > > I wonder if somewhere a signal handler is missing somewhere in one >>> of the >>> > > `run --local` or `run --raw`. I tried to follow all the paths through >>> > from >>> > > ui to sig handlers but got stuck in a tiwsty maze of classes. (and >>> was >>> > > attempting to do it just from reading the code)? >>> > > >>> > > >>> > > > On 8 Jan 2018, at 13:15, Driesprong, Fokko <[email protected]> >>> > wrote: >>> > > > >>> > > > Yes, for Spark this should work. Depending on the operator and the >>> > > > implementation: >>> > > > https://emea01.safelinks.protection.outlook.com/?url= >>> > https%3A%2F%2Fgithub.com%2Fapache%2Fincubator-airflow% >>> > 2Fblob%2F&data=01%7C01%7CEKC%40novozymes.com% >>> > 7C4d2e0189d7e44025c48608d559fa5913%7C43d5f49ee03a4d22a2285684196b >>> > b001%7C0&sdata=z1p%2FFlZ2cgPo%2BVK9sQJoDDJbKBEIdslsxtGOP%2F% >>> > 2B7g0M%3D&reserved=0 >>> > > 3e6babe8ed8f8f281b67aa3f4e03bf3cfc1bcbaa/airflow/contrib/ >>> > > hooks/spark_submit_hook.py#L412-L428 >>> > > > >>> > > > However this is a big change in behaviour. I'm curious about the >>> > opinion >>> > > of >>> > > > others. >>> > > > >>> > > > Cheers, >>> > > > Fokko >>> > > > >>> > > > >>> > > > 2018-01-08 14:12 GMT+01:00 Milan van der Meer < >>> > > > [email protected]>: >>> > > > >>> > > >> Any help? :) >>> > > >> >>> > > >> On Thu, Dec 14, 2017 at 8:12 PM, Milan van der Meer < >>> > > >> [email protected]> wrote: >>> > > >> >>> > > >>> I recently openend the following PR: https://emea01.safelinks. >>> > protection.outlook.com/?url=https%3A%2F%2Fgithub.com% >>> > 2Fapache%2F&data=01%7C01%7CEKC%40novozymes.com% >>> > 7C4d2e0189d7e44025c48608d559fa5913%7C43d5f49ee03a4d22a2285684196b >>> > b001%7C0&sdata=7AHCTsBnf%2F9nW0IY4B5ef3zfeY%2FlrH86WXwCt9YPwiU%3D& >>> > reserved=0 >>> > > >>> incubator-airflow/pull/2877 >>> > > >>> >>> > > >>> The problem is that on_kill is not called for operators when you >>> > clear >>> > > a >>> > > >>> task from the UI. >>> > > >>> Thats problematic when working with ex. spark clusters as the >>> jobs on >>> > > the >>> > > >>> cluster need to be killed. >>> > > >>> >>> > > >>> The issue is in the core code of Airflow and Im not familiar >>> enough >>> > > with >>> > > >>> the inner workings there. So I could use some directions on this >>> one >>> > > from >>> > > >>> people who are familiar. >>> > > >>> >>> > > >>> For more info, check out the PR. >>> > > >>> >>> > > >>> Kind regards, >>> > > >>> Milan >>> > > >>> >>> > > >> >>> > > >> >>> > > >> >>> > > >> -- >>> > > >> >>> > > >> *Milan van der Meer* >>> > > >> >>> > > >> *Real**Impact* Analytics *| *Big Data Consultant >>> > > >> https://emea01.safelinks.protection.outlook.com/?url= >>> > www.realimpactanalytics.com&data=01%7C01%7CEKC%40novozymes.com% >>> > 7C4d2e0189d7e44025c48608d559fa5913%7C43d5f49ee03a4d22a2285684196b >>> > b001%7C0&sdata=863cz5%2FF3LXZh52xM3pD0ORkew1PI8Q1Bt% >>> > 2B2pRiayWI%3D&reserved=0 >>> > > >> >>> > > >> *BE *+32 498 45 96 22 <+32%20498%2045%2096%2022> >>> <0032498459622>* | Skype *milan.vandermeer.ria >>> > > >> >>> > > >>> > > >>> > >>> > >>> > -- >>> > >>> > *Milan van der Meer* >>> > >>> > *Real**Impact* Analytics *| *Big Data Consultant >>> > https://emea01.safelinks.protection.outlook.com/?url= >>> > www.realimpactanalytics.com&data=01%7C01%7CEKC%40novozymes.com% >>> > 7C4d2e0189d7e44025c48608d559fa5913%7C43d5f49ee03a4d22a2285684196b >>> > b001%7C0&sdata=863cz5%2FF3LXZh52xM3pD0ORkew1PI8Q1Bt% >>> > 2B2pRiayWI%3D&reserved=0 >>> > >>> > *BE *+32 498 45 96 22 <+32%20498%2045%2096%2022> <0032498459622>* | >>> Skype *milan.vandermeer.ria >>> > >>> >> >> >> >> -- >> >> *Milan van der Meer* >> >> *Real**Impact* Analytics *| *Big Data Consultant >> www.realimpactanalytics.com >> >> *BE *+32 498 45 96 22 <0032498459622>* | Skype *milan.vandermeer.ria >> > > > > -- > > *Milan van der Meer* > > *Real**Impact* Analytics *| *Big Data Consultant > www.realimpactanalytics.com > > *BE *+32 498 45 96 22 <0032498459622>* | Skype *milan.vandermeer.ria > -- *Milan van der Meer* *Real**Impact* Analytics *| *Big Data Consultant www.realimpactanalytics.com *BE *+32 498 45 96 22 <0032498459622>* | Skype *milan.vandermeer.ria
