Re: Master is broken
Hi Kaxil, On Tue, Jun 12, 2018 at 5:12 PM, Naik Kaxil wrote: > > I have merged the PR that reverts the commit that broke the CI. Waiting > for the CI result. > > I see master is green again! Thanks :) Cheers, -- Gerardo Curiel // https://gerar.do
Re: Master is broken
Hi Fokko, I've been keeping track of the airflow builds just recently. So far I've seen TravisCI failing because of HTTP errors coming from Ubuntu repos, and maybe a few timeouts from tar downloads. But the recent ones were definitely related to bad merges. It seems to me that they can be identified easily: * If it happens in less than 10 minutes, flaky infrastructure * If it happens after 10 minutes (unless it's an integration test), then it's an actual bad merge Checking the build shouldn't take more than 2 minutes. In https://github.com/apache/incubator-airflow/pull/3393 I'm trying to address the flaky infrastructure by (among other things) reducing the number of downloads the build makes. Several small downloads have a higher probability of failure than a few big downloads. We would still need to fix these flaky tests though. Cheers, On Tue, Jun 12, 2018 at 4:54 PM, Driesprong, Fokko wrote: > Hi Gerardo, > > I totally agree that when master turns red, we should stop merging and fix > the build or revert the commit that broke the build. > > I think one of the underlying problems is having flaky tests, I tried to > fix a few of those, but they are quite persistent. Sometimes it is hard to > indentify if it is just a flaky test or if you really broke something. > > Cheers, Fokko > > Cheers, Fokko > > Op di 12 jun. 2018 om 07:37 schreef Daniel Imberman < > daniel.imber...@gmail.com> > > > +1 for merge blocking hooks. It would be great to have safety knowing > that > > any commit I revert to will still pass tests (for rebase testing, etc) > > > > On Mon, Jun 11, 2018 at 10:23 PM Alex Tronchin-James 949-412-7220 > > <(949)%20412-7220> wrote: > > > > > Could we adopt some sort of merge-blocking hook that prohibits merge of > > PRs > > > failing unit tests? My team has such an approach at work and it reduces > > the > > > volume of breakage quite a bit. The only time we experience problems > now > > is > > > where our unit test coverage is poor, but we improve the coverage every > > > time a breaking PR shows up. If our goal is to harden airflow for > ongoing > > > functionality with reduced breakage, this would be one good way to get > > > there. > > > > > > On Mon, Jun 11, 2018 at 7:55 PM Gerardo Curiel > wrote: > > > > > > > Hi folks, > > > > > > > > The master branch has been broken for a couple of days already. But > > that > > > > hasn't stopped the project from merging pull requests. As time passes > > by, > > > > it gets hard to identify what change caused the breakage. And of > > course, > > > > fixing it might cause conflicts with the changes introduced by the > > merged > > > > PRs. > > > > > > > > It seems to me that there should be some sort of process or > guidelines > > in > > > > place to avoid this sort of situations. "Don't merge if master is > red" > > > > seems like a reasonable option. > > > > > > > > If this guideline sounds obvious enough that it shouldn't be spelled > > out > > > in > > > > the commiters' documentation, then that's fine, but it hasn't been > > > followed > > > > recently. > > > > > > > > Cheers, > > > > > > > > -- > > > > Gerardo Curiel // https://gerar.do > > > > > > > > > > -- Gerardo Curiel // https://gerar.do
Re: Master is broken
Hi Gerado, As Fokko mentioned, sometimes the CI breaks because of some flaky test and sometime just fails due to connectivity issues while downloading required packages. If a certain commits break the master, we try to fix it asap. I have merged the PR that reverts the commit that broke the CI. Waiting for the CI result. Regards, Kaxil On 12/06/2018, 07:55, "Driesprong, Fokko" wrote: Hi Gerardo, I totally agree that when master turns red, we should stop merging and fix the build or revert the commit that broke the build. I think one of the underlying problems is having flaky tests, I tried to fix a few of those, but they are quite persistent. Sometimes it is hard to indentify if it is just a flaky test or if you really broke something. Cheers, Fokko Cheers, Fokko Op di 12 jun. 2018 om 07:37 schreef Daniel Imberman < daniel.imber...@gmail.com> > +1 for merge blocking hooks. It would be great to have safety knowing that > any commit I revert to will still pass tests (for rebase testing, etc) > > On Mon, Jun 11, 2018 at 10:23 PM Alex Tronchin-James 949-412-7220 > <(949)%20412-7220> wrote: > > > Could we adopt some sort of merge-blocking hook that prohibits merge of > PRs > > failing unit tests? My team has such an approach at work and it reduces > the > > volume of breakage quite a bit. The only time we experience problems now > is > > where our unit test coverage is poor, but we improve the coverage every > > time a breaking PR shows up. If our goal is to harden airflow for ongoing > > functionality with reduced breakage, this would be one good way to get > > there. > > > > On Mon, Jun 11, 2018 at 7:55 PM Gerardo Curiel wrote: > > > > > Hi folks, > > > > > > The master branch has been broken for a couple of days already. But > that > > > hasn't stopped the project from merging pull requests. As time passes > by, > > > it gets hard to identify what change caused the breakage. And of > course, > > > fixing it might cause conflicts with the changes introduced by the > merged > > > PRs. > > > > > > It seems to me that there should be some sort of process or guidelines > in > > > place to avoid this sort of situations. "Don't merge if master is red" > > > seems like a reasonable option. > > > > > > If this guideline sounds obvious enough that it shouldn't be spelled > out > > in > > > the commiters' documentation, then that's fine, but it hasn't been > > followed > > > recently. > > > > > > Cheers, > > > > > > -- > > > Gerardo Curiel // https://gerar.do > > > > > > Kaxil Naik Data Reply 2nd Floor, Nova South 160 Victoria Street, Westminster London SW1E 5LB - UK phone: +44 (0)20 7730 6000 k.n...@reply.com www.reply.com
Re: Master is broken
Hi Gerardo, I totally agree that when master turns red, we should stop merging and fix the build or revert the commit that broke the build. I think one of the underlying problems is having flaky tests, I tried to fix a few of those, but they are quite persistent. Sometimes it is hard to indentify if it is just a flaky test or if you really broke something. Cheers, Fokko Cheers, Fokko Op di 12 jun. 2018 om 07:37 schreef Daniel Imberman < daniel.imber...@gmail.com> > +1 for merge blocking hooks. It would be great to have safety knowing that > any commit I revert to will still pass tests (for rebase testing, etc) > > On Mon, Jun 11, 2018 at 10:23 PM Alex Tronchin-James 949-412-7220 > <(949)%20412-7220> wrote: > > > Could we adopt some sort of merge-blocking hook that prohibits merge of > PRs > > failing unit tests? My team has such an approach at work and it reduces > the > > volume of breakage quite a bit. The only time we experience problems now > is > > where our unit test coverage is poor, but we improve the coverage every > > time a breaking PR shows up. If our goal is to harden airflow for ongoing > > functionality with reduced breakage, this would be one good way to get > > there. > > > > On Mon, Jun 11, 2018 at 7:55 PM Gerardo Curiel wrote: > > > > > Hi folks, > > > > > > The master branch has been broken for a couple of days already. But > that > > > hasn't stopped the project from merging pull requests. As time passes > by, > > > it gets hard to identify what change caused the breakage. And of > course, > > > fixing it might cause conflicts with the changes introduced by the > merged > > > PRs. > > > > > > It seems to me that there should be some sort of process or guidelines > in > > > place to avoid this sort of situations. "Don't merge if master is red" > > > seems like a reasonable option. > > > > > > If this guideline sounds obvious enough that it shouldn't be spelled > out > > in > > > the commiters' documentation, then that's fine, but it hasn't been > > followed > > > recently. > > > > > > Cheers, > > > > > > -- > > > Gerardo Curiel // https://gerar.do > > > > > >
