You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Jarek Potiuk <Ja...@polidea.com> on 2020/01/13 21:21:11 UTC
Reverting [AIRFLOW-1467] Dynamic pooling
Hello everyone,
Without asking I quickly reverted the
https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
<https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic pooling via
allowing tasks to use more than one pool slot (depending upon the need))
as it had duplicated heads in sqlite migrations - clashing with an earlier
change
https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
added in
this PR https://github.com/apache/airflow/pull/6489: [AIRFLOW-4026]
<https://issues.apache.org/jira/browse/AIRFLOW-4026>
The problem was that both created independently SQL Alchemy migration heads
and we had a test failing because of that in master. The problem was that
the AIRFLOW-1467 was not rebased to the latest master before merging
(otherwise our tests detect this problem).
I will let the author of AIRFLOW-1467 how to fix it.
J.
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by Jarek Potiuk <Ja...@polidea.com>.
Thanks for contributing it Bin!
śr., 22 sty 2020, 07:03 użytkownik Xinbin Huang <bi...@gmail.com>
napisał:
> Just an update to this. The feature has been added to the master branch
> using the BoringCyborg. Hopefully, we can say goodbye to multiple migration
> heads from now on.
>
> Thanks @kaxil @jarek for the help during the process!
>
> Bin
>
> On Tue, Jan 14, 2020 at 12:29 PM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
> > Few more hints:
> >
> > - example how to see if any file in "airflow/migrations" folder has
> > been modified - you can see how Kaxil implemented labeler.js.
> > - I believe in order to check if the PR needs rebase you need to check
> > if the first (or last?) commit in the PR has parent = current master
> of the
> > repo. It might be a bit tricky I noticed that recently in Github they
> > apparently create different commit from your original one when you
> create
> > PR from a fork, so best to test it will be to make a fork of your fork
> > (inception!) and make PR to your repo from the 2nd fork :).
> >
> >
> > On Tue, Jan 14, 2020 at 9:19 PM Jarek Potiuk <Ja...@polidea.com>
> > wrote:
> >
> >> Cool!
> >>
> >>
> >> - Fork Kaxil's boring-cyborg: https://github.com/kaxil/boring-cyborg
> -
> >> you will create PRs from there
> >> - Setup environment. It's easy and fairly self-detected if you use
> >> IntelliJ or VSCode. Even if not, it boils down to running `npm
> install`.
> >> - run 'npm run dev'. It will run the app and it will automatically
> >> setup tunnel to your application with smee.io and you will get a
> >> unique URL that you can use when you create your application.
> Similar to :
> >>
> >> [image: Screenshot 2020-01-14 at 20.59.11.png]
> >>
> >> - You can also run it in IntelliJ/VScode using the standard way
> >> applications are run (this has the advantage that it works with the
> >> built-in debugger). For example in IntelliJ you need to create this
> >> configuration:
> >>
> >> [image: Screenshot 2020-01-14 at 21.03.22.png]
> >>
> >> - If you run it with "Run configuration" IntelliJ or similar in
> >> VSCode you should be able to use "Run" or "Debug" - in the latter
> case you
> >> will be able to set breakpoints and debug with debugger (super
> useful).
> >> - Create (if you do not have it) your own Fork of Airflow (you will
> >> need it to test the probot)
> >> - Install boring-cyborg in your own Airflow fork following
> >> https://probot.github.io/docs/development/#configuring-a-github-app
> >> - At this stage the application should be ready - whenever you open
> >> new PRs in your fork (internal PRs - to your own repo not to Apache
> >> Airflow), update them etc, you should start receiving calls to your
> >> application and you should be able to debug the app etc.
> >> - Look at the examples implemented by Kaxil and me - they should be
> >> fairly clear to start from and do similar implementation. The probot
> >> functionalities are in "lib" folder. You need to add your new file to
> >> index.js to get it working
> >> - Whenever you add a new configuration - you need to modify it and
> >> push to master of your Airflow's fork.
> >> - For interaction with github (read PRs/commits/etc) you use octokit
> >> js rest library (documentation here:
> >> https://octokit.github.io/rest.js/) I found it easiest to do some
> >> live-debugging and looking at the responses I get to understand how
> I can
> >> use the data)
> >> - Enjoy coding!
> >> - Follow step-by-step instructions here on how to make PR when you
> >> are ready :) :
> >> https://github.com/kaxil/boring-cyborg/blob/master/CONTRIBUTING.md
> >>
> >> I hope it's helpful :)
> >>
> >> J.
> >>
> >> On Tue, Jan 14, 2020 at 7:40 PM Xinbin Huang <bi...@gmail.com>
> >> wrote:
> >>
> >>> Hi Jarek,
> >>>
> >>> I would like to take a try into this. Where should I get started?
> >>>
> >>> Thanks
> >>> Bin
> >>>
> >>> On Tue, Jan 14, 2020 at 1:28 AM Jarek Potiuk <Jarek.Potiuk@polidea.com
> >
> >>> wrote:
> >>>
> >>> > Absolutely - it's great that we have the checks in master :).
> >>> >
> >>> > But I am thinking about some solutions already. We actually can do
> >>> > something using Kaxil's probot. While preventing merges for all PRs
> >>> that
> >>> > have not been rebased to latest master is quite an overkill, but I
> >>> think we
> >>> > can prevent merging PRs that have not been rebased AND have some
> >>> > modifications in "airflow/migrations".
> >>> >
> >>> > It's rather easy to add such features to probot I'd say. Maybe
> someone
> >>> > would like to learn how to write probots and would like to add such
> >>> check?
> >>> > I am happy to provide mentoring and guidance on how to build/test it
> >>> (it's
> >>> > rather easy once you have it setup).
> >>> >
> >>> > J.
> >>> >
> >>> > On Tue, Jan 14, 2020 at 9:51 AM Driesprong, Fokko
> <fokko@driesprong.frl
> >>> >
> >>> > wrote:
> >>> >
> >>> > > Hi Jarek,
> >>> > >
> >>> > > Hopefully, this doesn't happen too often since the tables should be
> >>> > slowly
> >>> > > changing. The solution here is to ask the contributors to rebase
> >>> their PR
> >>> > > more often.
> >>> > >
> >>> > > I'm happy that this is being caught now, and we don't have multiple
> >>> heads
> >>> > > as before :-)
> >>> > >
> >>> > > Cheers, Fokko
> >>> > >
> >>> > > Op ma 13 jan. 2020 om 22:23 schreef Jarek Potiuk <
> >>> > Jarek.Potiuk@polidea.com
> >>> > > >:
> >>> > >
> >>> > > > BTW. We'll have to find a better solution to prevent it as it has
> >>> > > happened
> >>> > > > in the past. I will think about it :). Any ideas are welcome :).
> >>> > > >
> >>> > > > On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <
> >>> > Jarek.Potiuk@polidea.com>
> >>> > > > wrote:
> >>> > > >
> >>> > > > > Hello everyone,
> >>> > > > >
> >>> > > > > Without asking I quickly reverted the
> >>> > > > > https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
> >>> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic
> >>> > pooling
> >>> > > > via
> >>> > > > > allowing tasks to use more than one pool slot (depending upon
> the
> >>> > > need))
> >>> > > > > as it had duplicated heads in sqlite migrations - clashing with
> >>> an
> >>> > > > earlier
> >>> > > > > change
> >>> > > > >
> >>> > > >
> >>> > >
> >>> >
> >>>
> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
> >>> > > > added in
> >>> > > > > this PR https://github.com/apache/airflow/pull/6489:
> >>> [AIRFLOW-4026]
> >>> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-4026>
> >>> > > > >
> >>> > > > > The problem was that both created independently SQL Alchemy
> >>> migration
> >>> > > > > heads and we had a test failing because of that in master. The
> >>> > problem
> >>> > > > was
> >>> > > > > that the AIRFLOW-1467 was not rebased to the latest master
> before
> >>> > > merging
> >>> > > > > (otherwise our tests detect this problem).
> >>> > > > >
> >>> > > > > I will let the author of AIRFLOW-1467 how to fix it.
> >>> > > > >
> >>> > > > > J.
> >>> > > > >
> >>> > > > > --
> >>> > > > >
> >>> > > > > Jarek Potiuk
> >>> > > > > Polidea <https://www.polidea.com/> | Principal Software
> Engineer
> >>> > > > >
> >>> > > > > M: +48 660 796 129 <+48660796129>
> >>> > > > > [image: Polidea] <https://www.polidea.com/>
> >>> > > > >
> >>> > > > >
> >>> > > >
> >>> > > > --
> >>> > > >
> >>> > > > Jarek Potiuk
> >>> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >>> > > >
> >>> > > > M: +48 660 796 129 <+48660796129>
> >>> > > > [image: Polidea] <https://www.polidea.com/>
> >>> > > >
> >>> > >
> >>> >
> >>> >
> >>> > --
> >>> >
> >>> > Jarek Potiuk
> >>> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >>> >
> >>> > M: +48 660 796 129 <+48660796129>
> >>> > [image: Polidea] <https://www.polidea.com/>
> >>> >
> >>>
> >>
> >>
> >> --
> >>
> >> Jarek Potiuk
> >> Polidea <https://www.polidea.com/> | Principal Software Engineer
> >>
> >> M: +48 660 796 129 <+48660796129>
> >> [image: Polidea] <https://www.polidea.com/>
> >>
> >>
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
> >
> >
>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by Xinbin Huang <bi...@gmail.com>.
Just an update to this. The feature has been added to the master branch
using the BoringCyborg. Hopefully, we can say goodbye to multiple migration
heads from now on.
Thanks @kaxil @jarek for the help during the process!
Bin
On Tue, Jan 14, 2020 at 12:29 PM Jarek Potiuk <Ja...@polidea.com>
wrote:
> Few more hints:
>
> - example how to see if any file in "airflow/migrations" folder has
> been modified - you can see how Kaxil implemented labeler.js.
> - I believe in order to check if the PR needs rebase you need to check
> if the first (or last?) commit in the PR has parent = current master of the
> repo. It might be a bit tricky I noticed that recently in Github they
> apparently create different commit from your original one when you create
> PR from a fork, so best to test it will be to make a fork of your fork
> (inception!) and make PR to your repo from the 2nd fork :).
>
>
> On Tue, Jan 14, 2020 at 9:19 PM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
>> Cool!
>>
>>
>> - Fork Kaxil's boring-cyborg: https://github.com/kaxil/boring-cyborg -
>> you will create PRs from there
>> - Setup environment. It's easy and fairly self-detected if you use
>> IntelliJ or VSCode. Even if not, it boils down to running `npm install`.
>> - run 'npm run dev'. It will run the app and it will automatically
>> setup tunnel to your application with smee.io and you will get a
>> unique URL that you can use when you create your application. Similar to :
>>
>> [image: Screenshot 2020-01-14 at 20.59.11.png]
>>
>> - You can also run it in IntelliJ/VScode using the standard way
>> applications are run (this has the advantage that it works with the
>> built-in debugger). For example in IntelliJ you need to create this
>> configuration:
>>
>> [image: Screenshot 2020-01-14 at 21.03.22.png]
>>
>> - If you run it with "Run configuration" IntelliJ or similar in
>> VSCode you should be able to use "Run" or "Debug" - in the latter case you
>> will be able to set breakpoints and debug with debugger (super useful).
>> - Create (if you do not have it) your own Fork of Airflow (you will
>> need it to test the probot)
>> - Install boring-cyborg in your own Airflow fork following
>> https://probot.github.io/docs/development/#configuring-a-github-app
>> - At this stage the application should be ready - whenever you open
>> new PRs in your fork (internal PRs - to your own repo not to Apache
>> Airflow), update them etc, you should start receiving calls to your
>> application and you should be able to debug the app etc.
>> - Look at the examples implemented by Kaxil and me - they should be
>> fairly clear to start from and do similar implementation. The probot
>> functionalities are in "lib" folder. You need to add your new file to
>> index.js to get it working
>> - Whenever you add a new configuration - you need to modify it and
>> push to master of your Airflow's fork.
>> - For interaction with github (read PRs/commits/etc) you use octokit
>> js rest library (documentation here:
>> https://octokit.github.io/rest.js/) I found it easiest to do some
>> live-debugging and looking at the responses I get to understand how I can
>> use the data)
>> - Enjoy coding!
>> - Follow step-by-step instructions here on how to make PR when you
>> are ready :) :
>> https://github.com/kaxil/boring-cyborg/blob/master/CONTRIBUTING.md
>>
>> I hope it's helpful :)
>>
>> J.
>>
>> On Tue, Jan 14, 2020 at 7:40 PM Xinbin Huang <bi...@gmail.com>
>> wrote:
>>
>>> Hi Jarek,
>>>
>>> I would like to take a try into this. Where should I get started?
>>>
>>> Thanks
>>> Bin
>>>
>>> On Tue, Jan 14, 2020 at 1:28 AM Jarek Potiuk <Ja...@polidea.com>
>>> wrote:
>>>
>>> > Absolutely - it's great that we have the checks in master :).
>>> >
>>> > But I am thinking about some solutions already. We actually can do
>>> > something using Kaxil's probot. While preventing merges for all PRs
>>> that
>>> > have not been rebased to latest master is quite an overkill, but I
>>> think we
>>> > can prevent merging PRs that have not been rebased AND have some
>>> > modifications in "airflow/migrations".
>>> >
>>> > It's rather easy to add such features to probot I'd say. Maybe someone
>>> > would like to learn how to write probots and would like to add such
>>> check?
>>> > I am happy to provide mentoring and guidance on how to build/test it
>>> (it's
>>> > rather easy once you have it setup).
>>> >
>>> > J.
>>> >
>>> > On Tue, Jan 14, 2020 at 9:51 AM Driesprong, Fokko <fokko@driesprong.frl
>>> >
>>> > wrote:
>>> >
>>> > > Hi Jarek,
>>> > >
>>> > > Hopefully, this doesn't happen too often since the tables should be
>>> > slowly
>>> > > changing. The solution here is to ask the contributors to rebase
>>> their PR
>>> > > more often.
>>> > >
>>> > > I'm happy that this is being caught now, and we don't have multiple
>>> heads
>>> > > as before :-)
>>> > >
>>> > > Cheers, Fokko
>>> > >
>>> > > Op ma 13 jan. 2020 om 22:23 schreef Jarek Potiuk <
>>> > Jarek.Potiuk@polidea.com
>>> > > >:
>>> > >
>>> > > > BTW. We'll have to find a better solution to prevent it as it has
>>> > > happened
>>> > > > in the past. I will think about it :). Any ideas are welcome :).
>>> > > >
>>> > > > On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <
>>> > Jarek.Potiuk@polidea.com>
>>> > > > wrote:
>>> > > >
>>> > > > > Hello everyone,
>>> > > > >
>>> > > > > Without asking I quickly reverted the
>>> > > > > https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
>>> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic
>>> > pooling
>>> > > > via
>>> > > > > allowing tasks to use more than one pool slot (depending upon the
>>> > > need))
>>> > > > > as it had duplicated heads in sqlite migrations - clashing with
>>> an
>>> > > > earlier
>>> > > > > change
>>> > > > >
>>> > > >
>>> > >
>>> >
>>> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
>>> > > > added in
>>> > > > > this PR https://github.com/apache/airflow/pull/6489:
>>> [AIRFLOW-4026]
>>> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-4026>
>>> > > > >
>>> > > > > The problem was that both created independently SQL Alchemy
>>> migration
>>> > > > > heads and we had a test failing because of that in master. The
>>> > problem
>>> > > > was
>>> > > > > that the AIRFLOW-1467 was not rebased to the latest master before
>>> > > merging
>>> > > > > (otherwise our tests detect this problem).
>>> > > > >
>>> > > > > I will let the author of AIRFLOW-1467 how to fix it.
>>> > > > >
>>> > > > > J.
>>> > > > >
>>> > > > > --
>>> > > > >
>>> > > > > Jarek Potiuk
>>> > > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
>>> > > > >
>>> > > > > M: +48 660 796 129 <+48660796129>
>>> > > > > [image: Polidea] <https://www.polidea.com/>
>>> > > > >
>>> > > > >
>>> > > >
>>> > > > --
>>> > > >
>>> > > > Jarek Potiuk
>>> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
>>> > > >
>>> > > > M: +48 660 796 129 <+48660796129>
>>> > > > [image: Polidea] <https://www.polidea.com/>
>>> > > >
>>> > >
>>> >
>>> >
>>> > --
>>> >
>>> > Jarek Potiuk
>>> > Polidea <https://www.polidea.com/> | Principal Software Engineer
>>> >
>>> > M: +48 660 796 129 <+48660796129>
>>> > [image: Polidea] <https://www.polidea.com/>
>>> >
>>>
>>
>>
>> --
>>
>> Jarek Potiuk
>> Polidea <https://www.polidea.com/> | Principal Software Engineer
>>
>> M: +48 660 796 129 <+48660796129>
>> [image: Polidea] <https://www.polidea.com/>
>>
>>
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by Jarek Potiuk <Ja...@polidea.com>.
Few more hints:
- example how to see if any file in "airflow/migrations" folder has been
modified - you can see how Kaxil implemented labeler.js.
- I believe in order to check if the PR needs rebase you need to check
if the first (or last?) commit in the PR has parent = current master of the
repo. It might be a bit tricky I noticed that recently in Github they
apparently create different commit from your original one when you create
PR from a fork, so best to test it will be to make a fork of your fork
(inception!) and make PR to your repo from the 2nd fork :).
On Tue, Jan 14, 2020 at 9:19 PM Jarek Potiuk <Ja...@polidea.com>
wrote:
> Cool!
>
>
> - Fork Kaxil's boring-cyborg: https://github.com/kaxil/boring-cyborg -
> you will create PRs from there
> - Setup environment. It's easy and fairly self-detected if you use
> IntelliJ or VSCode. Even if not, it boils down to running `npm install`.
> - run 'npm run dev'. It will run the app and it will automatically
> setup tunnel to your application with smee.io and you will get a
> unique URL that you can use when you create your application. Similar to :
>
> [image: Screenshot 2020-01-14 at 20.59.11.png]
>
> - You can also run it in IntelliJ/VScode using the standard way
> applications are run (this has the advantage that it works with the
> built-in debugger). For example in IntelliJ you need to create this
> configuration:
>
> [image: Screenshot 2020-01-14 at 21.03.22.png]
>
> - If you run it with "Run configuration" IntelliJ or similar in VSCode
> you should be able to use "Run" or "Debug" - in the latter case you will be
> able to set breakpoints and debug with debugger (super useful).
> - Create (if you do not have it) your own Fork of Airflow (you will
> need it to test the probot)
> - Install boring-cyborg in your own Airflow fork following
> https://probot.github.io/docs/development/#configuring-a-github-app
> - At this stage the application should be ready - whenever you open
> new PRs in your fork (internal PRs - to your own repo not to Apache
> Airflow), update them etc, you should start receiving calls to your
> application and you should be able to debug the app etc.
> - Look at the examples implemented by Kaxil and me - they should be
> fairly clear to start from and do similar implementation. The probot
> functionalities are in "lib" folder. You need to add your new file to
> index.js to get it working
> - Whenever you add a new configuration - you need to modify it and
> push to master of your Airflow's fork.
> - For interaction with github (read PRs/commits/etc) you use octokit
> js rest library (documentation here: https://octokit.github.io/rest.js/)
> I found it easiest to do some live-debugging and looking at the responses I
> get to understand how I can use the data)
> - Enjoy coding!
> - Follow step-by-step instructions here on how to make PR when you are
> ready :) :
> https://github.com/kaxil/boring-cyborg/blob/master/CONTRIBUTING.md
>
> I hope it's helpful :)
>
> J.
>
> On Tue, Jan 14, 2020 at 7:40 PM Xinbin Huang <bi...@gmail.com>
> wrote:
>
>> Hi Jarek,
>>
>> I would like to take a try into this. Where should I get started?
>>
>> Thanks
>> Bin
>>
>> On Tue, Jan 14, 2020 at 1:28 AM Jarek Potiuk <Ja...@polidea.com>
>> wrote:
>>
>> > Absolutely - it's great that we have the checks in master :).
>> >
>> > But I am thinking about some solutions already. We actually can do
>> > something using Kaxil's probot. While preventing merges for all PRs that
>> > have not been rebased to latest master is quite an overkill, but I
>> think we
>> > can prevent merging PRs that have not been rebased AND have some
>> > modifications in "airflow/migrations".
>> >
>> > It's rather easy to add such features to probot I'd say. Maybe someone
>> > would like to learn how to write probots and would like to add such
>> check?
>> > I am happy to provide mentoring and guidance on how to build/test it
>> (it's
>> > rather easy once you have it setup).
>> >
>> > J.
>> >
>> > On Tue, Jan 14, 2020 at 9:51 AM Driesprong, Fokko <fokko@driesprong.frl
>> >
>> > wrote:
>> >
>> > > Hi Jarek,
>> > >
>> > > Hopefully, this doesn't happen too often since the tables should be
>> > slowly
>> > > changing. The solution here is to ask the contributors to rebase
>> their PR
>> > > more often.
>> > >
>> > > I'm happy that this is being caught now, and we don't have multiple
>> heads
>> > > as before :-)
>> > >
>> > > Cheers, Fokko
>> > >
>> > > Op ma 13 jan. 2020 om 22:23 schreef Jarek Potiuk <
>> > Jarek.Potiuk@polidea.com
>> > > >:
>> > >
>> > > > BTW. We'll have to find a better solution to prevent it as it has
>> > > happened
>> > > > in the past. I will think about it :). Any ideas are welcome :).
>> > > >
>> > > > On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <
>> > Jarek.Potiuk@polidea.com>
>> > > > wrote:
>> > > >
>> > > > > Hello everyone,
>> > > > >
>> > > > > Without asking I quickly reverted the
>> > > > > https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
>> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic
>> > pooling
>> > > > via
>> > > > > allowing tasks to use more than one pool slot (depending upon the
>> > > need))
>> > > > > as it had duplicated heads in sqlite migrations - clashing with an
>> > > > earlier
>> > > > > change
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
>> > > > added in
>> > > > > this PR https://github.com/apache/airflow/pull/6489:
>> [AIRFLOW-4026]
>> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-4026>
>> > > > >
>> > > > > The problem was that both created independently SQL Alchemy
>> migration
>> > > > > heads and we had a test failing because of that in master. The
>> > problem
>> > > > was
>> > > > > that the AIRFLOW-1467 was not rebased to the latest master before
>> > > merging
>> > > > > (otherwise our tests detect this problem).
>> > > > >
>> > > > > I will let the author of AIRFLOW-1467 how to fix it.
>> > > > >
>> > > > > J.
>> > > > >
>> > > > > --
>> > > > >
>> > > > > Jarek Potiuk
>> > > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
>> > > > >
>> > > > > M: +48 660 796 129 <+48660796129>
>> > > > > [image: Polidea] <https://www.polidea.com/>
>> > > > >
>> > > > >
>> > > >
>> > > > --
>> > > >
>> > > > Jarek Potiuk
>> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
>> > > >
>> > > > M: +48 660 796 129 <+48660796129>
>> > > > [image: Polidea] <https://www.polidea.com/>
>> > > >
>> > >
>> >
>> >
>> > --
>> >
>> > Jarek Potiuk
>> > Polidea <https://www.polidea.com/> | Principal Software Engineer
>> >
>> > M: +48 660 796 129 <+48660796129>
>> > [image: Polidea] <https://www.polidea.com/>
>> >
>>
>
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by Jarek Potiuk <Ja...@polidea.com>.
Cool!
- Fork Kaxil's boring-cyborg: https://github.com/kaxil/boring-cyborg -
you will create PRs from there
- Setup environment. It's easy and fairly self-detected if you use
IntelliJ or VSCode. Even if not, it boils down to running `npm install`.
- run 'npm run dev'. It will run the app and it will automatically setup
tunnel to your application with smee.io and you will get a unique URL
that you can use when you create your application. Similar to :
[image: Screenshot 2020-01-14 at 20.59.11.png]
- You can also run it in IntelliJ/VScode using the standard way
applications are run (this has the advantage that it works with the
built-in debugger). For example in IntelliJ you need to create this
configuration:
[image: Screenshot 2020-01-14 at 21.03.22.png]
- If you run it with "Run configuration" IntelliJ or similar in VSCode
you should be able to use "Run" or "Debug" - in the latter case you will be
able to set breakpoints and debug with debugger (super useful).
- Create (if you do not have it) your own Fork of Airflow (you will need
it to test the probot)
- Install boring-cyborg in your own Airflow fork following
https://probot.github.io/docs/development/#configuring-a-github-app
- At this stage the application should be ready - whenever you open new
PRs in your fork (internal PRs - to your own repo not to Apache Airflow),
update them etc, you should start receiving calls to your application and
you should be able to debug the app etc.
- Look at the examples implemented by Kaxil and me - they should be
fairly clear to start from and do similar implementation. The probot
functionalities are in "lib" folder. You need to add your new file to
index.js to get it working
- Whenever you add a new configuration - you need to modify it and push
to master of your Airflow's fork.
- For interaction with github (read PRs/commits/etc) you use octokit js
rest library (documentation here: https://octokit.github.io/rest.js/) I
found it easiest to do some live-debugging and looking at the responses I
get to understand how I can use the data)
- Enjoy coding!
- Follow step-by-step instructions here on how to make PR when you are
ready :) :
https://github.com/kaxil/boring-cyborg/blob/master/CONTRIBUTING.md
I hope it's helpful :)
J.
On Tue, Jan 14, 2020 at 7:40 PM Xinbin Huang <bi...@gmail.com> wrote:
> Hi Jarek,
>
> I would like to take a try into this. Where should I get started?
>
> Thanks
> Bin
>
> On Tue, Jan 14, 2020 at 1:28 AM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
> > Absolutely - it's great that we have the checks in master :).
> >
> > But I am thinking about some solutions already. We actually can do
> > something using Kaxil's probot. While preventing merges for all PRs that
> > have not been rebased to latest master is quite an overkill, but I think
> we
> > can prevent merging PRs that have not been rebased AND have some
> > modifications in "airflow/migrations".
> >
> > It's rather easy to add such features to probot I'd say. Maybe someone
> > would like to learn how to write probots and would like to add such
> check?
> > I am happy to provide mentoring and guidance on how to build/test it
> (it's
> > rather easy once you have it setup).
> >
> > J.
> >
> > On Tue, Jan 14, 2020 at 9:51 AM Driesprong, Fokko <fo...@driesprong.frl>
> > wrote:
> >
> > > Hi Jarek,
> > >
> > > Hopefully, this doesn't happen too often since the tables should be
> > slowly
> > > changing. The solution here is to ask the contributors to rebase their
> PR
> > > more often.
> > >
> > > I'm happy that this is being caught now, and we don't have multiple
> heads
> > > as before :-)
> > >
> > > Cheers, Fokko
> > >
> > > Op ma 13 jan. 2020 om 22:23 schreef Jarek Potiuk <
> > Jarek.Potiuk@polidea.com
> > > >:
> > >
> > > > BTW. We'll have to find a better solution to prevent it as it has
> > > happened
> > > > in the past. I will think about it :). Any ideas are welcome :).
> > > >
> > > > On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <
> > Jarek.Potiuk@polidea.com>
> > > > wrote:
> > > >
> > > > > Hello everyone,
> > > > >
> > > > > Without asking I quickly reverted the
> > > > > https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic
> > pooling
> > > > via
> > > > > allowing tasks to use more than one pool slot (depending upon the
> > > need))
> > > > > as it had duplicated heads in sqlite migrations - clashing with an
> > > > earlier
> > > > > change
> > > > >
> > > >
> > >
> >
> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
> > > > added in
> > > > > this PR https://github.com/apache/airflow/pull/6489:
> [AIRFLOW-4026]
> > > > > <https://issues.apache.org/jira/browse/AIRFLOW-4026>
> > > > >
> > > > > The problem was that both created independently SQL Alchemy
> migration
> > > > > heads and we had a test failing because of that in master. The
> > problem
> > > > was
> > > > > that the AIRFLOW-1467 was not rebased to the latest master before
> > > merging
> > > > > (otherwise our tests detect this problem).
> > > > >
> > > > > I will let the author of AIRFLOW-1467 how to fix it.
> > > > >
> > > > > J.
> > > > >
> > > > > --
> > > > >
> > > > > Jarek Potiuk
> > > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > > > >
> > > > > M: +48 660 796 129 <+48660796129>
> > > > > [image: Polidea] <https://www.polidea.com/>
> > > > >
> > > > >
> > > >
> > > > --
> > > >
> > > > Jarek Potiuk
> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > > >
> > > > M: +48 660 796 129 <+48660796129>
> > > > [image: Polidea] <https://www.polidea.com/>
> > > >
> > >
> >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
> >
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by Xinbin Huang <bi...@gmail.com>.
Hi Jarek,
I would like to take a try into this. Where should I get started?
Thanks
Bin
On Tue, Jan 14, 2020 at 1:28 AM Jarek Potiuk <Ja...@polidea.com>
wrote:
> Absolutely - it's great that we have the checks in master :).
>
> But I am thinking about some solutions already. We actually can do
> something using Kaxil's probot. While preventing merges for all PRs that
> have not been rebased to latest master is quite an overkill, but I think we
> can prevent merging PRs that have not been rebased AND have some
> modifications in "airflow/migrations".
>
> It's rather easy to add such features to probot I'd say. Maybe someone
> would like to learn how to write probots and would like to add such check?
> I am happy to provide mentoring and guidance on how to build/test it (it's
> rather easy once you have it setup).
>
> J.
>
> On Tue, Jan 14, 2020 at 9:51 AM Driesprong, Fokko <fo...@driesprong.frl>
> wrote:
>
> > Hi Jarek,
> >
> > Hopefully, this doesn't happen too often since the tables should be
> slowly
> > changing. The solution here is to ask the contributors to rebase their PR
> > more often.
> >
> > I'm happy that this is being caught now, and we don't have multiple heads
> > as before :-)
> >
> > Cheers, Fokko
> >
> > Op ma 13 jan. 2020 om 22:23 schreef Jarek Potiuk <
> Jarek.Potiuk@polidea.com
> > >:
> >
> > > BTW. We'll have to find a better solution to prevent it as it has
> > happened
> > > in the past. I will think about it :). Any ideas are welcome :).
> > >
> > > On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <
> Jarek.Potiuk@polidea.com>
> > > wrote:
> > >
> > > > Hello everyone,
> > > >
> > > > Without asking I quickly reverted the
> > > > https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
> > > > <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic
> pooling
> > > via
> > > > allowing tasks to use more than one pool slot (depending upon the
> > need))
> > > > as it had duplicated heads in sqlite migrations - clashing with an
> > > earlier
> > > > change
> > > >
> > >
> >
> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
> > > added in
> > > > this PR https://github.com/apache/airflow/pull/6489: [AIRFLOW-4026]
> > > > <https://issues.apache.org/jira/browse/AIRFLOW-4026>
> > > >
> > > > The problem was that both created independently SQL Alchemy migration
> > > > heads and we had a test failing because of that in master. The
> problem
> > > was
> > > > that the AIRFLOW-1467 was not rebased to the latest master before
> > merging
> > > > (otherwise our tests detect this problem).
> > > >
> > > > I will let the author of AIRFLOW-1467 how to fix it.
> > > >
> > > > J.
> > > >
> > > > --
> > > >
> > > > Jarek Potiuk
> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > > >
> > > > M: +48 660 796 129 <+48660796129>
> > > > [image: Polidea] <https://www.polidea.com/>
> > > >
> > > >
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> > >
> >
>
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by Jarek Potiuk <Ja...@polidea.com>.
Absolutely - it's great that we have the checks in master :).
But I am thinking about some solutions already. We actually can do
something using Kaxil's probot. While preventing merges for all PRs that
have not been rebased to latest master is quite an overkill, but I think we
can prevent merging PRs that have not been rebased AND have some
modifications in "airflow/migrations".
It's rather easy to add such features to probot I'd say. Maybe someone
would like to learn how to write probots and would like to add such check?
I am happy to provide mentoring and guidance on how to build/test it (it's
rather easy once you have it setup).
J.
On Tue, Jan 14, 2020 at 9:51 AM Driesprong, Fokko <fo...@driesprong.frl>
wrote:
> Hi Jarek,
>
> Hopefully, this doesn't happen too often since the tables should be slowly
> changing. The solution here is to ask the contributors to rebase their PR
> more often.
>
> I'm happy that this is being caught now, and we don't have multiple heads
> as before :-)
>
> Cheers, Fokko
>
> Op ma 13 jan. 2020 om 22:23 schreef Jarek Potiuk <Jarek.Potiuk@polidea.com
> >:
>
> > BTW. We'll have to find a better solution to prevent it as it has
> happened
> > in the past. I will think about it :). Any ideas are welcome :).
> >
> > On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <Ja...@polidea.com>
> > wrote:
> >
> > > Hello everyone,
> > >
> > > Without asking I quickly reverted the
> > > https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
> > > <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic pooling
> > via
> > > allowing tasks to use more than one pool slot (depending upon the
> need))
> > > as it had duplicated heads in sqlite migrations - clashing with an
> > earlier
> > > change
> > >
> >
> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
> > added in
> > > this PR https://github.com/apache/airflow/pull/6489: [AIRFLOW-4026]
> > > <https://issues.apache.org/jira/browse/AIRFLOW-4026>
> > >
> > > The problem was that both created independently SQL Alchemy migration
> > > heads and we had a test failing because of that in master. The problem
> > was
> > > that the AIRFLOW-1467 was not rebased to the latest master before
> merging
> > > (otherwise our tests detect this problem).
> > >
> > > I will let the author of AIRFLOW-1467 how to fix it.
> > >
> > > J.
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> > >
> > >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
> >
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by "Driesprong, Fokko" <fo...@driesprong.frl>.
Hi Jarek,
Hopefully, this doesn't happen too often since the tables should be slowly
changing. The solution here is to ask the contributors to rebase their PR
more often.
I'm happy that this is being caught now, and we don't have multiple heads
as before :-)
Cheers, Fokko
Op ma 13 jan. 2020 om 22:23 schreef Jarek Potiuk <Ja...@polidea.com>:
> BTW. We'll have to find a better solution to prevent it as it has happened
> in the past. I will think about it :). Any ideas are welcome :).
>
> On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
> > Hello everyone,
> >
> > Without asking I quickly reverted the
> > https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
> > <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic pooling
> via
> > allowing tasks to use more than one pool slot (depending upon the need))
> > as it had duplicated heads in sqlite migrations - clashing with an
> earlier
> > change
> >
> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b
> added in
> > this PR https://github.com/apache/airflow/pull/6489: [AIRFLOW-4026]
> > <https://issues.apache.org/jira/browse/AIRFLOW-4026>
> >
> > The problem was that both created independently SQL Alchemy migration
> > heads and we had a test failing because of that in master. The problem
> was
> > that the AIRFLOW-1467 was not rebased to the latest master before merging
> > (otherwise our tests detect this problem).
> >
> > I will let the author of AIRFLOW-1467 how to fix it.
> >
> > J.
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
> >
> >
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
Re: Reverting [AIRFLOW-1467] Dynamic pooling
Posted by Jarek Potiuk <Ja...@polidea.com>.
BTW. We'll have to find a better solution to prevent it as it has happened
in the past. I will think about it :). Any ideas are welcome :).
On Mon, Jan 13, 2020 at 10:21 PM Jarek Potiuk <Ja...@polidea.com>
wrote:
> Hello everyone,
>
> Without asking I quickly reverted the
> https://github.com/apache/airflow/pull/6975 ([AIRFLOW-1467
> <https://issues.apache.org/jira/browse/AIRFLOW-1467>] Dynamic pooling via
> allowing tasks to use more than one pool slot (depending upon the need))
> as it had duplicated heads in sqlite migrations - clashing with an earlier
> change
> https://github.com/apache/airflow/commit/a7cacf593f5cf4bfc8b192b799aa2b14c96eac5b added in
> this PR https://github.com/apache/airflow/pull/6489: [AIRFLOW-4026]
> <https://issues.apache.org/jira/browse/AIRFLOW-4026>
>
> The problem was that both created independently SQL Alchemy migration
> heads and we had a test failing because of that in master. The problem was
> that the AIRFLOW-1467 was not rebased to the latest master before merging
> (otherwise our tests detect this problem).
>
> I will let the author of AIRFLOW-1467 how to fix it.
>
> J.
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>