Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Nov 1, 2016

What changes were proposed in this pull request?

Currently, Spark supports the test on Windows via AppVeyor but not it seems failing to download R 3.3.1 after R 3.3.2 is released. It downloads given R version after checking if that is the latest or not via http://rversions.r-pkg.org/r-release because the URL.

For example, the latest one has the URL as below:

https://cran.r-project.org/bin/windows/base/R-3.3.1-win.exe

and the old one has the URL as below.

https://cran.r-project.org/bin/windows/base/old/3.3.0/R-3.3.0-win.exe

The problem is, it seems (according to my observation, please check the main page[1]) the versions of R on Windows are not always synced with the latest versions. It seems R 3.3.2 for Windows does not exist.

So, it seems currently we can't download R 3.3.1 from the old repository[2] . So, currently, AppVeyor tries to find R 3.3.1 in the old repository[2] as 3.3.2 is released but it does not exist because it seems R 3.3.2 for Windows is not there and R 3.3.1 is still the latest.

Therefore, this PR proposes to lower the R version into 3.3.0 rather than rely on the latest as SparkR supports R 3.1+ if I recall correctly.

[1]https://cran.r-project.org
[2]https://cran.r-project.org/bin/windows/base/old

How was this patch tested?

Manually via AppVeyor CI - https://ci.appveyor.com/project/spark-test/spark/build/43-test

@HyukjinKwon
Copy link
Member Author

Build started: [SparkR] ALL PR-15709
Diff: master...spark-test:B0651ECA-1AB2-4452-89B7-A1BF7652113A

@HyukjinKwon
Copy link
Member Author

cc @felixcheung, @shivaram, @srowen and @wangmiao1981 (who I believe met this issue first).

@HyukjinKwon
Copy link
Member Author

Maybe, I might create a JIRA to describe the known issues with AppVeyor. (does it make sense?)

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 1, 2016

(cc @nchammas too who I also guess met this issue ahead. I would like to let him know.)

@shivaram
Copy link
Contributor

shivaram commented Nov 1, 2016

This approach is fine - The other thing we could do is just use the latest stable version as described in https://cloud.r-project.org/bin/windows/base/ - If you see the link at the bottom it says

Note to webmasters: A stable link which will redirect to the current Windows binary release is
https://cloud.r-project.org/bin/windows/base/release.htm

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 1, 2016

I see. That seems nice too but I hope the version is fixed if this one is also fine too..

I am worried if the version is suddenly bumped up and some tests are failed (at least, I was a bit upset when all other AppVeyor builds went suddenly failed). Maybe, I can try make the script take the version latest which uses that link.

I remember I met a incompatibility case between 3.1.x and 3.3.x when I used env (it seems it is immutable in 3.3.x but not in 3.1.x if I recall correctly) which showed a failure mark. It is good to find such cases ahead but it might be better if it does not affect other PRs.

(Not a strong opinion but I just wanted to let you know what I thought).

@SparkQA
Copy link

SparkQA commented Nov 1, 2016

Test build #67867 has finished for PR 15709 at commit 90fe001.

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

@shivaram
Copy link
Contributor

shivaram commented Nov 1, 2016

Yeah I'm fine with fixing a version - but as you said its sometimes helpful to find if we have some problems with a newly released R version. But yeah we should have a stable download URL whichever way we choose.

@HyukjinKwon
Copy link
Member Author

Oh wait. Please do not merge this.

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Nov 1, 2016

Oh, it seems R 3.3.2 for Windows is just right now released as well. It seems fine now - https://ci.appveyor.com/project/spark-test/spark/build/45-test1122
I will close this if the tests pass fine.

Maybe, we should take care of using the latest in the future. Thank you for quick responses @shivaram !

@HyukjinKwon
Copy link
Member Author

It seems fine now. Closing this.

@HyukjinKwon HyukjinKwon closed this Nov 1, 2016
ghost pushed a commit to dbtsai/spark that referenced this pull request Aug 8, 2017
… AppVeyor

## What changes were proposed in this pull request?

This PR proposes to use https://rversions.r-pkg.org/r-release-win instead of https://rversions.r-pkg.org/r-release to check R's version for Windows correctly.

We met a syncing problem with Windows release (see apache#15709) before. To cut this short, it was ...

- 3.3.2 release was released but not for Windows for few hours.
- `https://rversions.r-pkg.org/r-release` returns the latest as 3.3.2 and the download link for 3.3.1 becomes `windows/base/old` by our script
- 3.3.2 release for WIndows yet
- 3.3.1 is still not in `windows/base/old` but `windows/base` as the latest
- Failed to download with `windows/base/old` link and builds were broken

I believe this problem is not only what we met. Please see krlmlr/r-appveyor@01ce943 and also this `r-release-win` API came out between 3.3.1 and 3.3.2 (assuming to deal with this issue), please see `https://github.com/metacran/rversions.app/issues/2`.

Using this API will prevent the problem although it looks quite rare assuming from the commit logs in https://github.com/metacran/rversions.app/commits/master. After 3.3.2, both  `r-release-win` and `r-release` are being updated together.

## How was this patch tested?

AppVeyor tests.

Author: hyukjinkwon <gurwls223@gmail.com>

Closes apache#18859 from HyukjinKwon/use-reliable-link.
@HyukjinKwon HyukjinKwon deleted the SPARK-18190 branch January 2, 2018 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants