You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Wes McKinney <we...@gmail.com> on 2019/07/17 15:22:33 UTC

Caution about CI builds on personal forks

hi folks -- I noticed this last night on
https://github.com/apache/arrow/pull/4841 and it surprised me. Others
may not be aware.

We have been using builds on Appveyor and Travis CI to decide whether
to merge PRs. The trouble is these builds are not equivalent to the
builds that Travis runs inside the PR (using the apache/arrow build
queue). The differences are:

* They do not take into account changes in master (IOW to test if the
build works after `git merge`)
* They only test the latest commit versus the previous one in the branch

This latter item is insidious, because of the `detect-changes.py`
script. Suppose that you have a large PR that touches many components,
and you push a commit that only affects one of them. Then the
detect-changes.py script will cause Travis to only run builds for the
affected component in the most recent commit.

Here's an example of such a spurious build

https://travis-ci.org/wesm/arrow/builds/559745190

There are a few ways we can mitigate this last issue:

* If you need a faster build, you can squash your commits and rebase
on master before pushing to make sure that Travis "sees" everything.
Note this still carries risk of conflicting changes in master causing
a broken build post-merge

* We can change the Travis configuration to try to detect whether or
not we are testing a PR -- the detect-changes.py logic is really only
intended to speed up builds in apache/arrow

Overall, I think we need to accelerate our exodus from Travis CI since
it's hurting the project's productivity to be waiting so long on
builds. We've moved a couple of jobs to be Docker-based but we have
quite a lot more work to do to decouple ourselves.

Thanks
Wes

Re: Caution about CI builds on personal forks

Posted by Krisztián Szűcs <sz...@gmail.com>.
On Wed, Jul 17, 2019 at 5:55 PM Wes McKinney <we...@gmail.com> wrote:

> Presumably a migration away from Travis also means that we have to
> develop tools to allow contributors to test their patches outside of
> the GitHub pull request. If something is Docker-based, then it can be
> run locally, of course.
>
Ursabot can be hosted by anyone actually, here are the steps to run it
locally:
https://github.com/ursa-labs/ursabot#run-a-local-instance-of-ursabot

>
> We definitely can't persist under the current circumstances where
> builds take hours to even begin. Here's an example of a PR that was
> approved 3 hours ago but whose Travis builds only started about 10
> minutes ago (and will have to run for at least another 30-60 minutes)
>
> https://github.com/apache/arrow/pull/4894
>
> I think we need to get to an SLA where we're getting feedback on PRs
> in 90 minutes or less.
>
> On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> <ne...@gmail.com> wrote:
> >
> > Won't moving CI away from Travis to our own infrastructure mean that we
> > won't get any CI on our personal forks?
> >
> > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com>
> wrote:
> >
> > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com>
> wrote:
> > > >
> > > > hi folks -- I noticed this last night on
> > > > https://github.com/apache/arrow/pull/4841 and it surprised me.
> Others
> > > > may not be aware.
> > > >
> > > > We have been using builds on Appveyor and Travis CI to decide whether
> > > > to merge PRs. The trouble is these builds are not equivalent to the
> > > > builds that Travis runs inside the PR (using the apache/arrow build
> > > > queue). The differences are:
> > > >
> > >
> > > *missing crucial detail: "builds on personal forks"
> > >
> > > > * They do not take into account changes in master (IOW to test if the
> > > > build works after `git merge`)
> > > > * They only test the latest commit versus the previous one in the
> branch
> > > >
> > > > This latter item is insidious, because of the `detect-changes.py`
> > > > script. Suppose that you have a large PR that touches many
> components,
> > > > and you push a commit that only affects one of them. Then the
> > > > detect-changes.py script will cause Travis to only run builds for the
> > > > affected component in the most recent commit.
> > > >
> > > > Here's an example of such a spurious build
> > > >
> > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > >
> > > > There are a few ways we can mitigate this last issue:
> > > >
> > > > * If you need a faster build, you can squash your commits and rebase
> > > > on master before pushing to make sure that Travis "sees" everything.
> > > > Note this still carries risk of conflicting changes in master causing
> > > > a broken build post-merge
> > > >
> > > > * We can change the Travis configuration to try to detect whether or
> > > > not we are testing a PR -- the detect-changes.py logic is really only
> > > > intended to speed up builds in apache/arrow
> > > >
> > > > Overall, I think we need to accelerate our exodus from Travis CI
> since
> > > > it's hurting the project's productivity to be waiting so long on
> > > > builds. We've moved a couple of jobs to be Docker-based but we have
> > > > quite a lot more work to do to decouple ourselves.
> > > >
> > > > Thanks
> > > > Wes
> > >
>

Re: Caution about CI builds on personal forks

Posted by Krisztián Szűcs <sz...@gmail.com>.
Docker on Mac is able to run other archs as well, like aarch64.

On Wed, Jul 17, 2019 at 6:16 PM Uwe L. Korn <uw...@xhochy.com> wrote:

> Docker works well for all people on all OSes.
>
> Interesting will be Windows, OSX or aarch64 builds which require a special
> system.
>
> Uwe
>
> On Wed, Jul 17, 2019, at 6:11 PM, Antoine Pitrou wrote:
> >
> > I'm not sure how Docker will work for people not on Linux though?
> > (and/or for macOS builds)
> >
> > Regards
> >
> > Antoine.
> >
> >
> > On Wed, 17 Jul 2019 10:54:13 -0500
> > Wes McKinney <we...@gmail.com> wrote:
> >
> > > Presumably a migration away from Travis also means that we have to
> > > develop tools to allow contributors to test their patches outside of
> > > the GitHub pull request. If something is Docker-based, then it can be
> > > run locally, of course.
> > >
> > > We definitely can't persist under the current circumstances where
> > > builds take hours to even begin. Here's an example of a PR that was
> > > approved 3 hours ago but whose Travis builds only started about 10
> > > minutes ago (and will have to run for at least another 30-60 minutes)
> > >
> > > https://github.com/apache/arrow/pull/4894
> > >
> > > I think we need to get to an SLA where we're getting feedback on PRs
> > > in 90 minutes or less.
> > >
> > > On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> > > <ne...@gmail.com> wrote:
> > > >
> > > > Won't moving CI away from Travis to our own infrastructure mean that
> we
> > > > won't get any CI on our personal forks?
> > > >
> > > > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com>
> wrote:
> > > >
> > > > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com>
> wrote:
> > > > > >
> > > > > > hi folks -- I noticed this last night on
> > > > > > https://github.com/apache/arrow/pull/4841 and it surprised me.
> Others
> > > > > > may not be aware.
> > > > > >
> > > > > > We have been using builds on Appveyor and Travis CI to decide
> whether
> > > > > > to merge PRs. The trouble is these builds are not equivalent to
> the
> > > > > > builds that Travis runs inside the PR (using the apache/arrow
> build
> > > > > > queue). The differences are:
> > > > > >
> > > > >
> > > > > *missing crucial detail: "builds on personal forks"
> > > > >
> > > > > > * They do not take into account changes in master (IOW to test
> if the
> > > > > > build works after `git merge`)
> > > > > > * They only test the latest commit versus the previous one in
> the branch
> > > > > >
> > > > > > This latter item is insidious, because of the `detect-changes.py`
> > > > > > script. Suppose that you have a large PR that touches many
> components,
> > > > > > and you push a commit that only affects one of them. Then the
> > > > > > detect-changes.py script will cause Travis to only run builds
> for the
> > > > > > affected component in the most recent commit.
> > > > > >
> > > > > > Here's an example of such a spurious build
> > > > > >
> > > > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > > > >
> > > > > > There are a few ways we can mitigate this last issue:
> > > > > >
> > > > > > * If you need a faster build, you can squash your commits and
> rebase
> > > > > > on master before pushing to make sure that Travis "sees"
> everything.
> > > > > > Note this still carries risk of conflicting changes in master
> causing
> > > > > > a broken build post-merge
> > > > > >
> > > > > > * We can change the Travis configuration to try to detect
> whether or
> > > > > > not we are testing a PR -- the detect-changes.py logic is really
> only
> > > > > > intended to speed up builds in apache/arrow
> > > > > >
> > > > > > Overall, I think we need to accelerate our exodus from Travis CI
> since
> > > > > > it's hurting the project's productivity to be waiting so long on
> > > > > > builds. We've moved a couple of jobs to be Docker-based but we
> have
> > > > > > quite a lot more work to do to decouple ourselves.
> > > > > >
> > > > > > Thanks
> > > > > > Wes
> > > > >
> > >
> >
> >
> >
> >
>

Re: Caution about CI builds on personal forks

Posted by Wes McKinney <we...@gmail.com>.
Please keep in mind that any code transferred in from a third party
repository will probably need to go through the IP clearance process.
So if you do intend to move code, let's try to make it a one-time
thing so we can avoid this happening again in the future.

On Wed, Jul 17, 2019 at 12:45 PM Antoine Pitrou <so...@pitrou.net> wrote:
>
> On Wed, 17 Jul 2019 19:40:01 +0200
> Krisztián Szűcs <sz...@gmail.com> wrote:
> > >
> > > Hi Krisz -- we can't have primary CI configurations in a third party
> > > repository.
> > >
> > I have a solution in mind, most of the arrow related parts are clearly
> > separated, just need to organize to codebase accordingly.
> > (there are a couple of shared services like the github hook handling
> > which requires special treatment).
> > I can start to work on it from next week.
>
> It would be nice if you could write some kind of spec before starting
> to work on it IMO, so that we don't ask you to redo it differently :-)
>
> Regards
>
> Antoine.
>
>

Re: Caution about CI builds on personal forks

Posted by Antoine Pitrou <so...@pitrou.net>.
On Wed, 17 Jul 2019 19:40:01 +0200
Krisztián Szűcs <sz...@gmail.com> wrote:
> >
> > Hi Krisz -- we can't have primary CI configurations in a third party
> > repository.
> >  
> I have a solution in mind, most of the arrow related parts are clearly
> separated, just need to organize to codebase accordingly.
> (there are a couple of shared services like the github hook handling
> which requires special treatment).
> I can start to work on it from next week.

It would be nice if you could write some kind of spec before starting
to work on it IMO, so that we don't ask you to redo it differently :-)

Regards

Antoine.



Re: Caution about CI builds on personal forks

Posted by Krisztián Szűcs <sz...@gmail.com>.
>
> Hi Krisz -- we can't have primary CI configurations in a third party
> repository.
>
I have a solution in mind, most of the arrow related parts are clearly
separated, just need to organize to codebase accordingly.
(there are a couple of shared services like the github hook handling
which requires special treatment).
I can start to work on it from next week.

>
>  We need to figure out how to make 100% of builds runnable from the
> apache/arrow repository.
>
> On Wed, Jul 17, 2019, 12:29 PM Krisztián Szűcs <sz...@gmail.com>
> wrote:
>
> > Besides the builders listed at https://ci.ursalabs.org/#/builders
> > I've also migrated JS and R is in the queue
> > https://github.com/ursa-labs/ursabot/pull/135
> >
> > On Wed, Jul 17, 2019 at 6:23 PM Wes McKinney <we...@gmail.com>
> wrote:
> >
> > > Indeed, Docker runs on macOS and Windows
> > >
> > > In our Travis CI build matrix we have
> > >
> > > * 11 Linux builds
> > > * 3 macOS builds
> > >
> > > If we migrated just the Linux builds it might make the Travis wait
> > > times more acceptable in the short term
> > >
> > > On Wed, Jul 17, 2019 at 11:16 AM Uwe L. Korn <uw...@xhochy.com> wrote:
> > > >
> > > > Docker works well for all people on all OSes.
> > > >
> > > > Interesting will be Windows, OSX or aarch64 builds which require a
> > > special system.
> > > >
> > > > Uwe
> > > >
> > > > On Wed, Jul 17, 2019, at 6:11 PM, Antoine Pitrou wrote:
> > > > >
> > > > > I'm not sure how Docker will work for people not on Linux though?
> > > > > (and/or for macOS builds)
> > > > >
> > > > > Regards
> > > > >
> > > > > Antoine.
> > > > >
> > > > >
> > > > > On Wed, 17 Jul 2019 10:54:13 -0500
> > > > > Wes McKinney <we...@gmail.com> wrote:
> > > > >
> > > > > > Presumably a migration away from Travis also means that we have
> to
> > > > > > develop tools to allow contributors to test their patches outside
> > of
> > > > > > the GitHub pull request. If something is Docker-based, then it
> can
> > be
> > > > > > run locally, of course.
> > > > > >
> > > > > > We definitely can't persist under the current circumstances where
> > > > > > builds take hours to even begin. Here's an example of a PR that
> was
> > > > > > approved 3 hours ago but whose Travis builds only started about
> 10
> > > > > > minutes ago (and will have to run for at least another 30-60
> > minutes)
> > > > > >
> > > > > > https://github.com/apache/arrow/pull/4894
> > > > > >
> > > > > > I think we need to get to an SLA where we're getting feedback on
> > PRs
> > > > > > in 90 minutes or less.
> > > > > >
> > > > > > On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> > > > > > <ne...@gmail.com> wrote:
> > > > > > >
> > > > > > > Won't moving CI away from Travis to our own infrastructure mean
> > > that we
> > > > > > > won't get any CI on our personal forks?
> > > > > > >
> > > > > > > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <
> > wesmckinn@gmail.com>
> > > wrote:
> > > > > > >
> > > > > > > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <
> > > wesmckinn@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > hi folks -- I noticed this last night on
> > > > > > > > > https://github.com/apache/arrow/pull/4841 and it surprised
> > > me. Others
> > > > > > > > > may not be aware.
> > > > > > > > >
> > > > > > > > > We have been using builds on Appveyor and Travis CI to
> decide
> > > whether
> > > > > > > > > to merge PRs. The trouble is these builds are not
> equivalent
> > > to the
> > > > > > > > > builds that Travis runs inside the PR (using the
> apache/arrow
> > > build
> > > > > > > > > queue). The differences are:
> > > > > > > > >
> > > > > > > >
> > > > > > > > *missing crucial detail: "builds on personal forks"
> > > > > > > >
> > > > > > > > > * They do not take into account changes in master (IOW to
> > test
> > > if the
> > > > > > > > > build works after `git merge`)
> > > > > > > > > * They only test the latest commit versus the previous one
> in
> > > the branch
> > > > > > > > >
> > > > > > > > > This latter item is insidious, because of the
> > > `detect-changes.py`
> > > > > > > > > script. Suppose that you have a large PR that touches many
> > > components,
> > > > > > > > > and you push a commit that only affects one of them. Then
> the
> > > > > > > > > detect-changes.py script will cause Travis to only run
> builds
> > > for the
> > > > > > > > > affected component in the most recent commit.
> > > > > > > > >
> > > > > > > > > Here's an example of such a spurious build
> > > > > > > > >
> > > > > > > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > > > > > > >
> > > > > > > > > There are a few ways we can mitigate this last issue:
> > > > > > > > >
> > > > > > > > > * If you need a faster build, you can squash your commits
> and
> > > rebase
> > > > > > > > > on master before pushing to make sure that Travis "sees"
> > > everything.
> > > > > > > > > Note this still carries risk of conflicting changes in
> master
> > > causing
> > > > > > > > > a broken build post-merge
> > > > > > > > >
> > > > > > > > > * We can change the Travis configuration to try to detect
> > > whether or
> > > > > > > > > not we are testing a PR -- the detect-changes.py logic is
> > > really only
> > > > > > > > > intended to speed up builds in apache/arrow
> > > > > > > > >
> > > > > > > > > Overall, I think we need to accelerate our exodus from
> Travis
> > > CI since
> > > > > > > > > it's hurting the project's productivity to be waiting so
> long
> > > on
> > > > > > > > > builds. We've moved a couple of jobs to be Docker-based but
> > we
> > > have
> > > > > > > > > quite a lot more work to do to decouple ourselves.
> > > > > > > > >
> > > > > > > > > Thanks
> > > > > > > > > Wes
> > > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> >
>
On Wed, Jul 17, 2019 at 7:32 PM Wes McKinney <we...@gmail.com> wrote:

Re: Caution about CI builds on personal forks

Posted by Wes McKinney <we...@gmail.com>.
Hi Krisz -- we can't have primary CI configurations in a third party
repository.

 We need to figure out how to make 100% of builds runnable from the
apache/arrow repository.

On Wed, Jul 17, 2019, 12:29 PM Krisztián Szűcs <sz...@gmail.com>
wrote:

> Besides the builders listed at https://ci.ursalabs.org/#/builders
> I've also migrated JS and R is in the queue
> https://github.com/ursa-labs/ursabot/pull/135
>
> On Wed, Jul 17, 2019 at 6:23 PM Wes McKinney <we...@gmail.com> wrote:
>
> > Indeed, Docker runs on macOS and Windows
> >
> > In our Travis CI build matrix we have
> >
> > * 11 Linux builds
> > * 3 macOS builds
> >
> > If we migrated just the Linux builds it might make the Travis wait
> > times more acceptable in the short term
> >
> > On Wed, Jul 17, 2019 at 11:16 AM Uwe L. Korn <uw...@xhochy.com> wrote:
> > >
> > > Docker works well for all people on all OSes.
> > >
> > > Interesting will be Windows, OSX or aarch64 builds which require a
> > special system.
> > >
> > > Uwe
> > >
> > > On Wed, Jul 17, 2019, at 6:11 PM, Antoine Pitrou wrote:
> > > >
> > > > I'm not sure how Docker will work for people not on Linux though?
> > > > (and/or for macOS builds)
> > > >
> > > > Regards
> > > >
> > > > Antoine.
> > > >
> > > >
> > > > On Wed, 17 Jul 2019 10:54:13 -0500
> > > > Wes McKinney <we...@gmail.com> wrote:
> > > >
> > > > > Presumably a migration away from Travis also means that we have to
> > > > > develop tools to allow contributors to test their patches outside
> of
> > > > > the GitHub pull request. If something is Docker-based, then it can
> be
> > > > > run locally, of course.
> > > > >
> > > > > We definitely can't persist under the current circumstances where
> > > > > builds take hours to even begin. Here's an example of a PR that was
> > > > > approved 3 hours ago but whose Travis builds only started about 10
> > > > > minutes ago (and will have to run for at least another 30-60
> minutes)
> > > > >
> > > > > https://github.com/apache/arrow/pull/4894
> > > > >
> > > > > I think we need to get to an SLA where we're getting feedback on
> PRs
> > > > > in 90 minutes or less.
> > > > >
> > > > > On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> > > > > <ne...@gmail.com> wrote:
> > > > > >
> > > > > > Won't moving CI away from Travis to our own infrastructure mean
> > that we
> > > > > > won't get any CI on our personal forks?
> > > > > >
> > > > > > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <
> wesmckinn@gmail.com>
> > wrote:
> > > > > >
> > > > > > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <
> > wesmckinn@gmail.com> wrote:
> > > > > > > >
> > > > > > > > hi folks -- I noticed this last night on
> > > > > > > > https://github.com/apache/arrow/pull/4841 and it surprised
> > me. Others
> > > > > > > > may not be aware.
> > > > > > > >
> > > > > > > > We have been using builds on Appveyor and Travis CI to decide
> > whether
> > > > > > > > to merge PRs. The trouble is these builds are not equivalent
> > to the
> > > > > > > > builds that Travis runs inside the PR (using the apache/arrow
> > build
> > > > > > > > queue). The differences are:
> > > > > > > >
> > > > > > >
> > > > > > > *missing crucial detail: "builds on personal forks"
> > > > > > >
> > > > > > > > * They do not take into account changes in master (IOW to
> test
> > if the
> > > > > > > > build works after `git merge`)
> > > > > > > > * They only test the latest commit versus the previous one in
> > the branch
> > > > > > > >
> > > > > > > > This latter item is insidious, because of the
> > `detect-changes.py`
> > > > > > > > script. Suppose that you have a large PR that touches many
> > components,
> > > > > > > > and you push a commit that only affects one of them. Then the
> > > > > > > > detect-changes.py script will cause Travis to only run builds
> > for the
> > > > > > > > affected component in the most recent commit.
> > > > > > > >
> > > > > > > > Here's an example of such a spurious build
> > > > > > > >
> > > > > > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > > > > > >
> > > > > > > > There are a few ways we can mitigate this last issue:
> > > > > > > >
> > > > > > > > * If you need a faster build, you can squash your commits and
> > rebase
> > > > > > > > on master before pushing to make sure that Travis "sees"
> > everything.
> > > > > > > > Note this still carries risk of conflicting changes in master
> > causing
> > > > > > > > a broken build post-merge
> > > > > > > >
> > > > > > > > * We can change the Travis configuration to try to detect
> > whether or
> > > > > > > > not we are testing a PR -- the detect-changes.py logic is
> > really only
> > > > > > > > intended to speed up builds in apache/arrow
> > > > > > > >
> > > > > > > > Overall, I think we need to accelerate our exodus from Travis
> > CI since
> > > > > > > > it's hurting the project's productivity to be waiting so long
> > on
> > > > > > > > builds. We've moved a couple of jobs to be Docker-based but
> we
> > have
> > > > > > > > quite a lot more work to do to decouple ourselves.
> > > > > > > >
> > > > > > > > Thanks
> > > > > > > > Wes
> > > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> >
>

Re: Caution about CI builds on personal forks

Posted by Krisztián Szűcs <sz...@gmail.com>.
Besides the builders listed at https://ci.ursalabs.org/#/builders
I've also migrated JS and R is in the queue
https://github.com/ursa-labs/ursabot/pull/135

On Wed, Jul 17, 2019 at 6:23 PM Wes McKinney <we...@gmail.com> wrote:

> Indeed, Docker runs on macOS and Windows
>
> In our Travis CI build matrix we have
>
> * 11 Linux builds
> * 3 macOS builds
>
> If we migrated just the Linux builds it might make the Travis wait
> times more acceptable in the short term
>
> On Wed, Jul 17, 2019 at 11:16 AM Uwe L. Korn <uw...@xhochy.com> wrote:
> >
> > Docker works well for all people on all OSes.
> >
> > Interesting will be Windows, OSX or aarch64 builds which require a
> special system.
> >
> > Uwe
> >
> > On Wed, Jul 17, 2019, at 6:11 PM, Antoine Pitrou wrote:
> > >
> > > I'm not sure how Docker will work for people not on Linux though?
> > > (and/or for macOS builds)
> > >
> > > Regards
> > >
> > > Antoine.
> > >
> > >
> > > On Wed, 17 Jul 2019 10:54:13 -0500
> > > Wes McKinney <we...@gmail.com> wrote:
> > >
> > > > Presumably a migration away from Travis also means that we have to
> > > > develop tools to allow contributors to test their patches outside of
> > > > the GitHub pull request. If something is Docker-based, then it can be
> > > > run locally, of course.
> > > >
> > > > We definitely can't persist under the current circumstances where
> > > > builds take hours to even begin. Here's an example of a PR that was
> > > > approved 3 hours ago but whose Travis builds only started about 10
> > > > minutes ago (and will have to run for at least another 30-60 minutes)
> > > >
> > > > https://github.com/apache/arrow/pull/4894
> > > >
> > > > I think we need to get to an SLA where we're getting feedback on PRs
> > > > in 90 minutes or less.
> > > >
> > > > On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> > > > <ne...@gmail.com> wrote:
> > > > >
> > > > > Won't moving CI away from Travis to our own infrastructure mean
> that we
> > > > > won't get any CI on our personal forks?
> > > > >
> > > > > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com>
> wrote:
> > > > >
> > > > > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <
> wesmckinn@gmail.com> wrote:
> > > > > > >
> > > > > > > hi folks -- I noticed this last night on
> > > > > > > https://github.com/apache/arrow/pull/4841 and it surprised
> me. Others
> > > > > > > may not be aware.
> > > > > > >
> > > > > > > We have been using builds on Appveyor and Travis CI to decide
> whether
> > > > > > > to merge PRs. The trouble is these builds are not equivalent
> to the
> > > > > > > builds that Travis runs inside the PR (using the apache/arrow
> build
> > > > > > > queue). The differences are:
> > > > > > >
> > > > > >
> > > > > > *missing crucial detail: "builds on personal forks"
> > > > > >
> > > > > > > * They do not take into account changes in master (IOW to test
> if the
> > > > > > > build works after `git merge`)
> > > > > > > * They only test the latest commit versus the previous one in
> the branch
> > > > > > >
> > > > > > > This latter item is insidious, because of the
> `detect-changes.py`
> > > > > > > script. Suppose that you have a large PR that touches many
> components,
> > > > > > > and you push a commit that only affects one of them. Then the
> > > > > > > detect-changes.py script will cause Travis to only run builds
> for the
> > > > > > > affected component in the most recent commit.
> > > > > > >
> > > > > > > Here's an example of such a spurious build
> > > > > > >
> > > > > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > > > > >
> > > > > > > There are a few ways we can mitigate this last issue:
> > > > > > >
> > > > > > > * If you need a faster build, you can squash your commits and
> rebase
> > > > > > > on master before pushing to make sure that Travis "sees"
> everything.
> > > > > > > Note this still carries risk of conflicting changes in master
> causing
> > > > > > > a broken build post-merge
> > > > > > >
> > > > > > > * We can change the Travis configuration to try to detect
> whether or
> > > > > > > not we are testing a PR -- the detect-changes.py logic is
> really only
> > > > > > > intended to speed up builds in apache/arrow
> > > > > > >
> > > > > > > Overall, I think we need to accelerate our exodus from Travis
> CI since
> > > > > > > it's hurting the project's productivity to be waiting so long
> on
> > > > > > > builds. We've moved a couple of jobs to be Docker-based but we
> have
> > > > > > > quite a lot more work to do to decouple ourselves.
> > > > > > >
> > > > > > > Thanks
> > > > > > > Wes
> > > > > >
> > > >
> > >
> > >
> > >
> > >
>

Re: Caution about CI builds on personal forks

Posted by Wes McKinney <we...@gmail.com>.
Indeed, Docker runs on macOS and Windows

In our Travis CI build matrix we have

* 11 Linux builds
* 3 macOS builds

If we migrated just the Linux builds it might make the Travis wait
times more acceptable in the short term

On Wed, Jul 17, 2019 at 11:16 AM Uwe L. Korn <uw...@xhochy.com> wrote:
>
> Docker works well for all people on all OSes.
>
> Interesting will be Windows, OSX or aarch64 builds which require a special system.
>
> Uwe
>
> On Wed, Jul 17, 2019, at 6:11 PM, Antoine Pitrou wrote:
> >
> > I'm not sure how Docker will work for people not on Linux though?
> > (and/or for macOS builds)
> >
> > Regards
> >
> > Antoine.
> >
> >
> > On Wed, 17 Jul 2019 10:54:13 -0500
> > Wes McKinney <we...@gmail.com> wrote:
> >
> > > Presumably a migration away from Travis also means that we have to
> > > develop tools to allow contributors to test their patches outside of
> > > the GitHub pull request. If something is Docker-based, then it can be
> > > run locally, of course.
> > >
> > > We definitely can't persist under the current circumstances where
> > > builds take hours to even begin. Here's an example of a PR that was
> > > approved 3 hours ago but whose Travis builds only started about 10
> > > minutes ago (and will have to run for at least another 30-60 minutes)
> > >
> > > https://github.com/apache/arrow/pull/4894
> > >
> > > I think we need to get to an SLA where we're getting feedback on PRs
> > > in 90 minutes or less.
> > >
> > > On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> > > <ne...@gmail.com> wrote:
> > > >
> > > > Won't moving CI away from Travis to our own infrastructure mean that we
> > > > won't get any CI on our personal forks?
> > > >
> > > > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com> wrote:
> > > >
> > > > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com> wrote:
> > > > > >
> > > > > > hi folks -- I noticed this last night on
> > > > > > https://github.com/apache/arrow/pull/4841 and it surprised me. Others
> > > > > > may not be aware.
> > > > > >
> > > > > > We have been using builds on Appveyor and Travis CI to decide whether
> > > > > > to merge PRs. The trouble is these builds are not equivalent to the
> > > > > > builds that Travis runs inside the PR (using the apache/arrow build
> > > > > > queue). The differences are:
> > > > > >
> > > > >
> > > > > *missing crucial detail: "builds on personal forks"
> > > > >
> > > > > > * They do not take into account changes in master (IOW to test if the
> > > > > > build works after `git merge`)
> > > > > > * They only test the latest commit versus the previous one in the branch
> > > > > >
> > > > > > This latter item is insidious, because of the `detect-changes.py`
> > > > > > script. Suppose that you have a large PR that touches many components,
> > > > > > and you push a commit that only affects one of them. Then the
> > > > > > detect-changes.py script will cause Travis to only run builds for the
> > > > > > affected component in the most recent commit.
> > > > > >
> > > > > > Here's an example of such a spurious build
> > > > > >
> > > > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > > > >
> > > > > > There are a few ways we can mitigate this last issue:
> > > > > >
> > > > > > * If you need a faster build, you can squash your commits and rebase
> > > > > > on master before pushing to make sure that Travis "sees" everything.
> > > > > > Note this still carries risk of conflicting changes in master causing
> > > > > > a broken build post-merge
> > > > > >
> > > > > > * We can change the Travis configuration to try to detect whether or
> > > > > > not we are testing a PR -- the detect-changes.py logic is really only
> > > > > > intended to speed up builds in apache/arrow
> > > > > >
> > > > > > Overall, I think we need to accelerate our exodus from Travis CI since
> > > > > > it's hurting the project's productivity to be waiting so long on
> > > > > > builds. We've moved a couple of jobs to be Docker-based but we have
> > > > > > quite a lot more work to do to decouple ourselves.
> > > > > >
> > > > > > Thanks
> > > > > > Wes
> > > > >
> > >
> >
> >
> >
> >

Re: Caution about CI builds on personal forks

Posted by "Uwe L. Korn" <uw...@xhochy.com>.
Docker works well for all people on all OSes. 

Interesting will be Windows, OSX or aarch64 builds which require a special system. 

Uwe

On Wed, Jul 17, 2019, at 6:11 PM, Antoine Pitrou wrote:
> 
> I'm not sure how Docker will work for people not on Linux though?
> (and/or for macOS builds)
> 
> Regards
> 
> Antoine.
> 
> 
> On Wed, 17 Jul 2019 10:54:13 -0500
> Wes McKinney <we...@gmail.com> wrote:
> 
> > Presumably a migration away from Travis also means that we have to
> > develop tools to allow contributors to test their patches outside of
> > the GitHub pull request. If something is Docker-based, then it can be
> > run locally, of course.
> > 
> > We definitely can't persist under the current circumstances where
> > builds take hours to even begin. Here's an example of a PR that was
> > approved 3 hours ago but whose Travis builds only started about 10
> > minutes ago (and will have to run for at least another 30-60 minutes)
> > 
> > https://github.com/apache/arrow/pull/4894
> > 
> > I think we need to get to an SLA where we're getting feedback on PRs
> > in 90 minutes or less.
> > 
> > On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> > <ne...@gmail.com> wrote:
> > >
> > > Won't moving CI away from Travis to our own infrastructure mean that we
> > > won't get any CI on our personal forks?
> > >
> > > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com> wrote:
> > >  
> > > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com> wrote:  
> > > > >
> > > > > hi folks -- I noticed this last night on
> > > > > https://github.com/apache/arrow/pull/4841 and it surprised me. Others
> > > > > may not be aware.
> > > > >
> > > > > We have been using builds on Appveyor and Travis CI to decide whether
> > > > > to merge PRs. The trouble is these builds are not equivalent to the
> > > > > builds that Travis runs inside the PR (using the apache/arrow build
> > > > > queue). The differences are:
> > > > >  
> > > >
> > > > *missing crucial detail: "builds on personal forks"
> > > >  
> > > > > * They do not take into account changes in master (IOW to test if the
> > > > > build works after `git merge`)
> > > > > * They only test the latest commit versus the previous one in the branch
> > > > >
> > > > > This latter item is insidious, because of the `detect-changes.py`
> > > > > script. Suppose that you have a large PR that touches many components,
> > > > > and you push a commit that only affects one of them. Then the
> > > > > detect-changes.py script will cause Travis to only run builds for the
> > > > > affected component in the most recent commit.
> > > > >
> > > > > Here's an example of such a spurious build
> > > > >
> > > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > > >
> > > > > There are a few ways we can mitigate this last issue:
> > > > >
> > > > > * If you need a faster build, you can squash your commits and rebase
> > > > > on master before pushing to make sure that Travis "sees" everything.
> > > > > Note this still carries risk of conflicting changes in master causing
> > > > > a broken build post-merge
> > > > >
> > > > > * We can change the Travis configuration to try to detect whether or
> > > > > not we are testing a PR -- the detect-changes.py logic is really only
> > > > > intended to speed up builds in apache/arrow
> > > > >
> > > > > Overall, I think we need to accelerate our exodus from Travis CI since
> > > > > it's hurting the project's productivity to be waiting so long on
> > > > > builds. We've moved a couple of jobs to be Docker-based but we have
> > > > > quite a lot more work to do to decouple ourselves.
> > > > >
> > > > > Thanks
> > > > > Wes  
> > > >  
> > 
> 
> 
> 
>

Re: Caution about CI builds on personal forks

Posted by Antoine Pitrou <so...@pitrou.net>.
I'm not sure how Docker will work for people not on Linux though?
(and/or for macOS builds)

Regards

Antoine.


On Wed, 17 Jul 2019 10:54:13 -0500
Wes McKinney <we...@gmail.com> wrote:

> Presumably a migration away from Travis also means that we have to
> develop tools to allow contributors to test their patches outside of
> the GitHub pull request. If something is Docker-based, then it can be
> run locally, of course.
> 
> We definitely can't persist under the current circumstances where
> builds take hours to even begin. Here's an example of a PR that was
> approved 3 hours ago but whose Travis builds only started about 10
> minutes ago (and will have to run for at least another 30-60 minutes)
> 
> https://github.com/apache/arrow/pull/4894
> 
> I think we need to get to an SLA where we're getting feedback on PRs
> in 90 minutes or less.
> 
> On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
> <ne...@gmail.com> wrote:
> >
> > Won't moving CI away from Travis to our own infrastructure mean that we
> > won't get any CI on our personal forks?
> >
> > On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com> wrote:
> >  
> > > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com> wrote:  
> > > >
> > > > hi folks -- I noticed this last night on
> > > > https://github.com/apache/arrow/pull/4841 and it surprised me. Others
> > > > may not be aware.
> > > >
> > > > We have been using builds on Appveyor and Travis CI to decide whether
> > > > to merge PRs. The trouble is these builds are not equivalent to the
> > > > builds that Travis runs inside the PR (using the apache/arrow build
> > > > queue). The differences are:
> > > >  
> > >
> > > *missing crucial detail: "builds on personal forks"
> > >  
> > > > * They do not take into account changes in master (IOW to test if the
> > > > build works after `git merge`)
> > > > * They only test the latest commit versus the previous one in the branch
> > > >
> > > > This latter item is insidious, because of the `detect-changes.py`
> > > > script. Suppose that you have a large PR that touches many components,
> > > > and you push a commit that only affects one of them. Then the
> > > > detect-changes.py script will cause Travis to only run builds for the
> > > > affected component in the most recent commit.
> > > >
> > > > Here's an example of such a spurious build
> > > >
> > > > https://travis-ci.org/wesm/arrow/builds/559745190
> > > >
> > > > There are a few ways we can mitigate this last issue:
> > > >
> > > > * If you need a faster build, you can squash your commits and rebase
> > > > on master before pushing to make sure that Travis "sees" everything.
> > > > Note this still carries risk of conflicting changes in master causing
> > > > a broken build post-merge
> > > >
> > > > * We can change the Travis configuration to try to detect whether or
> > > > not we are testing a PR -- the detect-changes.py logic is really only
> > > > intended to speed up builds in apache/arrow
> > > >
> > > > Overall, I think we need to accelerate our exodus from Travis CI since
> > > > it's hurting the project's productivity to be waiting so long on
> > > > builds. We've moved a couple of jobs to be Docker-based but we have
> > > > quite a lot more work to do to decouple ourselves.
> > > >
> > > > Thanks
> > > > Wes  
> > >  
> 




Re: Caution about CI builds on personal forks

Posted by Wes McKinney <we...@gmail.com>.
Presumably a migration away from Travis also means that we have to
develop tools to allow contributors to test their patches outside of
the GitHub pull request. If something is Docker-based, then it can be
run locally, of course.

We definitely can't persist under the current circumstances where
builds take hours to even begin. Here's an example of a PR that was
approved 3 hours ago but whose Travis builds only started about 10
minutes ago (and will have to run for at least another 30-60 minutes)

https://github.com/apache/arrow/pull/4894

I think we need to get to an SLA where we're getting feedback on PRs
in 90 minutes or less.

On Wed, Jul 17, 2019 at 10:47 AM Neal Richardson
<ne...@gmail.com> wrote:
>
> Won't moving CI away from Travis to our own infrastructure mean that we
> won't get any CI on our personal forks?
>
> On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com> wrote:
>
> > On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com> wrote:
> > >
> > > hi folks -- I noticed this last night on
> > > https://github.com/apache/arrow/pull/4841 and it surprised me. Others
> > > may not be aware.
> > >
> > > We have been using builds on Appveyor and Travis CI to decide whether
> > > to merge PRs. The trouble is these builds are not equivalent to the
> > > builds that Travis runs inside the PR (using the apache/arrow build
> > > queue). The differences are:
> > >
> >
> > *missing crucial detail: "builds on personal forks"
> >
> > > * They do not take into account changes in master (IOW to test if the
> > > build works after `git merge`)
> > > * They only test the latest commit versus the previous one in the branch
> > >
> > > This latter item is insidious, because of the `detect-changes.py`
> > > script. Suppose that you have a large PR that touches many components,
> > > and you push a commit that only affects one of them. Then the
> > > detect-changes.py script will cause Travis to only run builds for the
> > > affected component in the most recent commit.
> > >
> > > Here's an example of such a spurious build
> > >
> > > https://travis-ci.org/wesm/arrow/builds/559745190
> > >
> > > There are a few ways we can mitigate this last issue:
> > >
> > > * If you need a faster build, you can squash your commits and rebase
> > > on master before pushing to make sure that Travis "sees" everything.
> > > Note this still carries risk of conflicting changes in master causing
> > > a broken build post-merge
> > >
> > > * We can change the Travis configuration to try to detect whether or
> > > not we are testing a PR -- the detect-changes.py logic is really only
> > > intended to speed up builds in apache/arrow
> > >
> > > Overall, I think we need to accelerate our exodus from Travis CI since
> > > it's hurting the project's productivity to be waiting so long on
> > > builds. We've moved a couple of jobs to be Docker-based but we have
> > > quite a lot more work to do to decouple ourselves.
> > >
> > > Thanks
> > > Wes
> >

Re: Caution about CI builds on personal forks

Posted by Neal Richardson <ne...@gmail.com>.
Won't moving CI away from Travis to our own infrastructure mean that we
won't get any CI on our personal forks?

On Wed, Jul 17, 2019 at 8:23 AM Wes McKinney <we...@gmail.com> wrote:

> On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com> wrote:
> >
> > hi folks -- I noticed this last night on
> > https://github.com/apache/arrow/pull/4841 and it surprised me. Others
> > may not be aware.
> >
> > We have been using builds on Appveyor and Travis CI to decide whether
> > to merge PRs. The trouble is these builds are not equivalent to the
> > builds that Travis runs inside the PR (using the apache/arrow build
> > queue). The differences are:
> >
>
> *missing crucial detail: "builds on personal forks"
>
> > * They do not take into account changes in master (IOW to test if the
> > build works after `git merge`)
> > * They only test the latest commit versus the previous one in the branch
> >
> > This latter item is insidious, because of the `detect-changes.py`
> > script. Suppose that you have a large PR that touches many components,
> > and you push a commit that only affects one of them. Then the
> > detect-changes.py script will cause Travis to only run builds for the
> > affected component in the most recent commit.
> >
> > Here's an example of such a spurious build
> >
> > https://travis-ci.org/wesm/arrow/builds/559745190
> >
> > There are a few ways we can mitigate this last issue:
> >
> > * If you need a faster build, you can squash your commits and rebase
> > on master before pushing to make sure that Travis "sees" everything.
> > Note this still carries risk of conflicting changes in master causing
> > a broken build post-merge
> >
> > * We can change the Travis configuration to try to detect whether or
> > not we are testing a PR -- the detect-changes.py logic is really only
> > intended to speed up builds in apache/arrow
> >
> > Overall, I think we need to accelerate our exodus from Travis CI since
> > it's hurting the project's productivity to be waiting so long on
> > builds. We've moved a couple of jobs to be Docker-based but we have
> > quite a lot more work to do to decouple ourselves.
> >
> > Thanks
> > Wes
>

Re: Caution about CI builds on personal forks

Posted by Wes McKinney <we...@gmail.com>.
On Wed, Jul 17, 2019 at 10:22 AM Wes McKinney <we...@gmail.com> wrote:
>
> hi folks -- I noticed this last night on
> https://github.com/apache/arrow/pull/4841 and it surprised me. Others
> may not be aware.
>
> We have been using builds on Appveyor and Travis CI to decide whether
> to merge PRs. The trouble is these builds are not equivalent to the
> builds that Travis runs inside the PR (using the apache/arrow build
> queue). The differences are:
>

*missing crucial detail: "builds on personal forks"

> * They do not take into account changes in master (IOW to test if the
> build works after `git merge`)
> * They only test the latest commit versus the previous one in the branch
>
> This latter item is insidious, because of the `detect-changes.py`
> script. Suppose that you have a large PR that touches many components,
> and you push a commit that only affects one of them. Then the
> detect-changes.py script will cause Travis to only run builds for the
> affected component in the most recent commit.
>
> Here's an example of such a spurious build
>
> https://travis-ci.org/wesm/arrow/builds/559745190
>
> There are a few ways we can mitigate this last issue:
>
> * If you need a faster build, you can squash your commits and rebase
> on master before pushing to make sure that Travis "sees" everything.
> Note this still carries risk of conflicting changes in master causing
> a broken build post-merge
>
> * We can change the Travis configuration to try to detect whether or
> not we are testing a PR -- the detect-changes.py logic is really only
> intended to speed up builds in apache/arrow
>
> Overall, I think we need to accelerate our exodus from Travis CI since
> it's hurting the project's productivity to be waiting so long on
> builds. We've moved a couple of jobs to be Docker-based but we have
> quite a lot more work to do to decouple ourselves.
>
> Thanks
> Wes