[SPARK-17200][PROJECT INFRA][BUILD][SPARKR] Automate building and testing on Windows (currently SparkR only) #14859

HyukjinKwon · 2016-08-29T09:08:31Z

What changes were proposed in this pull request?

This PR adds the build automation on Windows with AppVeyor CI tool.

Currently, this only runs the tests for SparkR as we have been having some issues with testing Windows-specific PRs (e.g. #14743 and #13165) and hard time to verify this.

One concern is, this build is dependent on steveloughran/winutils for pre-built Hadoop bin package (who is a Hadoop PMC member).

How was this patch tested?

Manually, https://ci.appveyor.com/project/HyukjinKwon/spark/build/88-SPARK-17200-build-profile
This takes roughly 40 mins.

~~Some tests are already being failed and this was found in #14743 (comment)~~. Fixed in SPARK-17339

HyukjinKwon · 2016-08-29T09:11:03Z

cc @rxin
@srowen (for build)
@JoshRosen (for project infra)
@dongjoon-hyun (who suggested AppVeyor CI)
@steveloughran (who is the author of winutils)
@felixcheung and @shivaram (for SparkR)

srowen · 2016-08-29T09:15:16Z

Hm, we also had Travis config that isn't used now, to try to add Java style checking. I can see the value in adding Windows testing, but here we have a third CI tool involved. I'm concerned that I for example wouldn't know how to maintain it.

I suppose we also need to decide if Windows is even supported? I don't think it is supported for development, certainly. For deployment -- best effort is my understanding, but may not work.

If this relies on a bunch of setup to run (including needing a sorta unofficial copy of Hadoop's winutils) then does testing it this way say much about how it works on Windows?

HyukjinKwon · 2016-08-29T09:27:32Z

Ah, I thought Windows is already officially supported assuming from this documentation https://github.com/apache/spark/blob/master/docs/index.md#downloading.

BTW, I do understand your concerns but I believe this will make easy to review Window-specific tests. I mean, at least, we can identify Windows-specific problems easily and as you already know, I believe it is hard to review the PRs for Windows-specific problems currently.

I wouldn't mind if this should be closed because, at least, I proposed a automated build on Windows here so reviewers can use this after manually (locally) merging this anyway.

My personal opinion is, though, to try to use this as it does not affect code-base or other builds.

srowen · 2016-08-29T09:44:41Z

Good point, I suppose there is a weak promise there that it runs on Windows.
Could anyone else who knows Windows weigh in? I assume @dongjoon-hyun is on board.

shivaram · 2016-08-29T16:59:34Z

Thanks @HyukjinKwon - I for one would at least like the SparkR client to work on Windows as I think there are quite a few R users who are on Windows.

@HyukjinKwon Could you add some documentation on how this build works, where we can check its status and what we can do to restart a build etc ?

JoshRosen · 2016-08-29T17:52:41Z

It looks like a few other Apache projects use AppVeyor and it looks like there's a designated process for asking INFRA to enable it for a GitHub mirror: https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift/history

For example, Thrift uses it: https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift

One concern of mine is whether the large number of Spark pull requests and commits would overwhelm Apache's AppVeyor account limits if every one of those builds were to be tested with AppVeyor.

Another minor concern of mine relates to GitHub build statuses: if AppVeyor reports its statuses via the GitHub commit status API but AMPLab Jenkins continues to report only via comments then the build status indicators on the pull request page would look confusing.

It would be cool if there was a way to have this enabled by default for the master branch but have it be opt-in for pull requests.

shivaram · 2016-08-29T18:02:09Z

Good point on only running this on the master branch. We could even run it periodically (say nightly) instead of on every commit.

felixcheung · 2016-08-29T19:13:34Z

dev/appveyor-install-dependencies.ps1

+Pop-Location
+
+# ========================== R
+$rVer = "3.3.0"


SparkQA · 2016-08-29T20:43:55Z

Test build #64556 has finished for PR 14859 at commit 1f23b05.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2016-08-30T02:42:02Z

Thanks all. Then, let me try to write some documentation for..

How this build works
Where we can check its status
What we can do to restart a build
Options to enable/disable the build for branch, PR and commits.

HyukjinKwon · 2016-08-30T04:01:40Z

How to set up

Sign-up https://ci.appveyor.com
Go to profile
Enable the link with GitHub
Add a project (Go to the PROJECTS menu and than add a new project)
Click Github project list and then click
Push any commit and check the build is running.
- Click PROJECTS menu
- Click Spark project

How to re-build/stop/check the builds

Click PROJECTS and then Spark project
- Click PROJECTS menu
- Click Spark project
Check build status
Re-build
Stop build

How this build works

Identically with Travis CI. Per-commit.

Options to enable/disable the build for branch, PR and commits.

Master branch only
- Click settings
- Set the branch to build as below:
Run the build periodically
Select commits to build (Some PRs)
- It seems we can filter some commits to run a build as described here
- If we enable Rolling build and filter the commits, I think it'd be okay maybe.
Disable builds for Pull Reqeusts (no PRs) - this is Github-side.
- Click Spark's Github Project settings
- Clieck Webhooks & services
- Clieck https://ci.appveyor.com/api/github/webhook (pull_request and push) - Disable webhook for each pull requests

BTW, there is a getting started guide here, https://www.appveyor.com/docs

HyukjinKwon · 2016-08-30T04:04:44Z

To cut it short, my suggestion is,

Filter commits via (maybe via message?) https://www.appveyor.com/docs/how-to/filtering-commits/
Enable Rolling build

... rolling builds make sure that only the most recent commit is built.
Run the build periodically as described above against master.

If it sounds good, I will go ahead and test. But before proceeding, it'd be great if I can hear other opinions. I think it'd be okay just to filter commits via message.

WDYT? - @srowen @JoshRosen @shivaram @felixcheung (I am on holiday until tomorrow. So I would like to test this environment quickly)

SparkQA · 2016-09-04T13:42:40Z

Test build #64922 has finished for PR 14859 at commit ccf176d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

shivaram · 2016-09-07T16:06:34Z

@HyukjinKwon Given that the build is green after #14960 is this good to go ? Any other comments @dongjoon-hyun @srowen

dongjoon-hyun · 2016-09-07T16:46:34Z

Hi, @shivaram and @HyukjinKwon . LGTM.

HyukjinKwon · 2016-09-07T22:51:02Z

Thank you! I also thimk it's good to go.

shivaram · 2016-09-07T23:02:48Z

appveyor.yml

+  - cmd: R -e "install.packages('testthat', repos='http://cran.us.r-project.org')"
+
+build_script:
+  - cmd: mvn -DskipTests -Psparkr -Phive -Phive-thriftserver package


Given that we are fixing the hadoop version that we download in install-dependencies.ps1 does it make sense to also pass in the same profile here ? i.e. -Phadoop-2.6 ?

shivaram · 2016-09-08T01:38:00Z

LGTM pending the inline comment about hadoop-2.6

HyukjinKwon · 2016-09-08T02:36:16Z

Thanks. FYI, I re-ran the test here - https://ci.appveyor.com/project/HyukjinKwon/spark/build/87-SPARK-17200-build-profile

HyukjinKwon · 2016-09-08T02:38:05Z

Please excuse me to upload image here (so that I can use it in the MD.)

shivaram · 2016-09-08T04:25:41Z

Thanks. I'll merge this once Jenkins passes

felixcheung · 2016-09-08T04:27:14Z

+@steveloughran since this is running his tool
How do we get alert when this fails?

felixcheung · 2016-09-08T04:28:33Z

appveyor.yml

+  # Install maven and dependencies
+  - ps: .\dev\appveyor-install-dependencies.ps1
+  # Required package for R unit tests
+  - cmd: R -e "install.packages('testthat', repos='http://cran.us.r-project.org')"


might want to include e1071, survival for a few more compatibility tests.
(see DESCRIPTION file)

we have been broken with newer versions of testthat before, not sure if we should fix the version we run with here to match Jenkins?

Ah, thanks! I will.

Thats a good point but its actually tricky to specify a version number using install.packages - FWIW on Jenkins I see

> packageVersion("testthat") [1] ‘1.0.2’

true - maybe just print out packageVersion into the log, in case it breaks

SparkQA · 2016-09-08T04:44:57Z

Test build #65072 has finished for PR 14859 at commit 8e1954b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-09-08T05:01:06Z

Test build #65073 has finished for PR 14859 at commit fe9419d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2016-09-08T06:44:02Z

I just added both survival and e1071. Also, it now prints the version for both.

I re-ran the test here - https://ci.appveyor.com/project/HyukjinKwon/spark/build/88-SPARK-17200-build-profile

HyukjinKwon · 2016-09-08T07:00:56Z

BTW, I already cc the author of winutils in #14859 (comment) :).

SparkQA · 2016-09-08T08:14:36Z

Test build #65083 has finished for PR 14859 at commit 9c06b92.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2016-09-08T08:21:22Z

retest this please

SparkQA · 2016-09-08T10:28:26Z

Test build #65095 has finished for PR 14859 at commit 9c06b92.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

shivaram · 2016-09-08T15:26:24Z

Alright, I'm merging this into master. Lets see how this goes in the next few days and we can fine tune this with some follow up PRs if necessary.

HyukjinKwon added 10 commits August 29, 2016 16:24

Appveyor SparkR Windows test draft

cf9f56c

Fix script path for R installation

b25eaba

Fix the name of script for R installation

2772002

Upgrade maven version to 3.3.9

6cb4416

Clean up and fix the path for Hadoop bin package

a2852a0

Merged dependecies installation

5aca104

Remove R installation script

fbcfe13

Clean up the dependencies installation script

f3eb163

Fix comment

fe95491

Uppercase for Maven in the comment

a8e74fc

HyukjinKwon added 6 commits August 29, 2016 19:12

Clean up and make pretty

69fd3f1

Fix typo in variable names

3a21367

Fix variable declaration

2b9af15

Fix R version to 3.3.0

cdb24a7

Remove meaningless CmdletBinding and Param

3988305

Consistent variable names

1f23b05

felixcheung reviewed Aug 29, 2016
View reviewed changes

HyukjinKwon added 2 commits August 30, 2016 13:43

Fix R version to 3.3.1 and minimize the codes

97f3ea7

Consistent downloading via Start-FileDownload

e7addc9

Fix typo in the guide line

ccf176d

shivaram reviewed Sep 7, 2016
View reviewed changes

Fetch upstream to pass R tests

27075de

Add -Phadoop-2.6 profile

8e1954b

HyukjinKwon added 2 commits September 8, 2016 11:47

Add the documenation for checking full details of failed tests

41dbdcf

Consistent newlines for each title

fe9419d

felixcheung reviewed Sep 8, 2016
View reviewed changes

Adds survival and e1071 for dependencies

9c06b92

asfgit closed this in 78d5d4d Sep 8, 2016

HyukjinKwon deleted the SPARK-17200-build branch January 2, 2018 03:39

[SPARK-17200][PROJECT INFRA][BUILD][SPARKR] Automate building and testing on Windows (currently SparkR only) #14859

[SPARK-17200][PROJECT INFRA][BUILD][SPARKR] Automate building and testing on Windows (currently SparkR only) #14859

Conversation

HyukjinKwon commented Aug 29, 2016 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

HyukjinKwon commented Aug 29, 2016 • edited Loading

srowen commented Aug 29, 2016

HyukjinKwon commented Aug 29, 2016 • edited Loading

srowen commented Aug 29, 2016

shivaram commented Aug 29, 2016

JoshRosen commented Aug 29, 2016

shivaram commented Aug 29, 2016

felixcheung Aug 29, 2016

Choose a reason for hiding this comment

SparkQA commented Aug 29, 2016

HyukjinKwon commented Aug 30, 2016

HyukjinKwon commented Aug 30, 2016 • edited Loading

How to set up

How to re-build/stop/check the builds

How this build works

Options to enable/disable the build for branch, PR and commits.

HyukjinKwon commented Aug 30, 2016 • edited Loading

SparkQA commented Sep 4, 2016

shivaram commented Sep 7, 2016

dongjoon-hyun commented Sep 7, 2016

HyukjinKwon commented Sep 7, 2016

shivaram Sep 7, 2016

Choose a reason for hiding this comment

shivaram commented Sep 8, 2016

HyukjinKwon commented Sep 8, 2016

HyukjinKwon commented Sep 8, 2016

shivaram commented Sep 8, 2016

felixcheung commented Sep 8, 2016 • edited Loading

felixcheung Sep 8, 2016

Choose a reason for hiding this comment

felixcheung Sep 8, 2016

Choose a reason for hiding this comment

HyukjinKwon Sep 8, 2016

Choose a reason for hiding this comment

shivaram Sep 8, 2016

Choose a reason for hiding this comment

felixcheung Sep 8, 2016

Choose a reason for hiding this comment

SparkQA commented Sep 8, 2016

SparkQA commented Sep 8, 2016

HyukjinKwon commented Sep 8, 2016

HyukjinKwon commented Sep 8, 2016

SparkQA commented Sep 8, 2016

HyukjinKwon commented Sep 8, 2016

SparkQA commented Sep 8, 2016

shivaram commented Sep 8, 2016

HyukjinKwon commented Aug 29, 2016 •

edited

Loading

HyukjinKwon commented Aug 29, 2016 •

edited

Loading

HyukjinKwon commented Aug 29, 2016 •

edited

Loading

HyukjinKwon commented Aug 30, 2016 •

edited

Loading

HyukjinKwon commented Aug 30, 2016 •

edited

Loading

felixcheung commented Sep 8, 2016 •

edited

Loading