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 <jarek.pot...@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 <jarek.pot...@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 <bin.huan...@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.pot...@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.pot...@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.pot...@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/> > >
