Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented May 20, 2016

What changes were proposed in this pull request?

This PR adds the description for running unit tests in Windows.

How was this patch tested?

On a bare machine (Window 7, 32bits), this was manually built and tested.

@HyukjinKwon HyukjinKwon changed the title [MI [MINOR][SPARKR][DOC] Add a description for running unit tests in Windows May 20, 2016
@HyukjinKwon
Copy link
Member Author

cc @sun-rui

@SparkQA
Copy link

SparkQA commented May 20, 2016

Test build #58963 has finished for PR 13217 at commit b62b631.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 20, 2016

Test build #58962 has finished for PR 13217 at commit acb9363.

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

R/WINDOWS.md Outdated
1. Set `HADOOP_HOME`.
2. Download `winutils.exe` and locate this in `$HADOOP_HOME/bin`.

It seems not requiring installing Hadoop but only this `winutils.exe`. It seems not included in Hadoop official binary releases so it should be built from source but it seems it is able to be downloaded from community (e.g. [steveloughran/winutils](https://github.com/steveloughran/winutils)).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @steveloughran for comment. I think the paragraph should start with "It is not included in the Hadoop binary releases, so .... However it is downloadable from, for example [...]"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I will wait for the comment and will fix.

Copy link
Contributor

@steveloughran steveloughran May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't recommend putting it under the root of the project, as that only complicates the source tree and path cleanup; an adjacent directory works. And I think you may find that HADOOP.DLL is needed in places, as there are some JNI calls related to local file access and permissions/ACLs

At some point we (the Hadoop team) may start releasing the Windows binaries direct. It's only avoided as it complicates the release process somewhat, though if it encourages testing it can only be encouraged.

I'd suggest the following text:


To run the SparkR unit tests on Windows, the following steps are required —assuming you are in the Spark root directory and do not have Apache Hadoop installed already:

  1. cd ..
  2. mkdir hadoop
  3. Download the relevant Hadoop bin package from steveloughran/winutils. While these are not official ASF artifacts, they are built from the ASF release git hashes by a Hadoop PMC member on a dedicated Windows VM.
  4. Install the files into hadoop\bin; make sure that winutils.exe and hadoop.dll are present.
  5. Set the environment variable HADOOP_HOME to the full path to the newly created hadoop directory.
  6. For further reading, consult Windows Problems on the Hadoop wiki

@steveloughran
Copy link
Contributor

  1. It's nice to see someone sitting down to deal with the windows test problem.
  2. Hadoop 2.8+ will fail meaningfully here, with an exception including a link to the Hadoop wiki entry HADOOP-10775. I've put a link to the entry in my suggested changes, so people trying to get things working have a direct link to that page, so can stay current with whatever changes are made (more docs, ASF official releases), ...
  3. Someone should really test the documentation by following it. I must excuse myself on the basis that my VM is set up and I don't want to break things.

@sun-rui
Copy link
Contributor

sun-rui commented May 20, 2016

wait to see if someone can test the documentation.

@SparkQA
Copy link

SparkQA commented May 21, 2016

Test build #59051 has finished for PR 13217 at commit aa2839c.

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

@HyukjinKwon
Copy link
Member Author

@sun-rui @steveloughran While it seems obviously better for someone to follow and test this, I wonder who is going to test this and leave some comments here.

I opened this PR during working on #13165, which I started to work on because #7025 is getting stale for several months. In this way, it seems nobody would want to try this.

(In that way, #13165 will also never be merged unless someone is trying to test. I would quit and close it if I should work on something which it seems will never be merged).

Maybe it would be better, at least for this one, to merge this for users/developers to follow this document to test, since it is a minor which does not affect the code base.

@sun-rui
Copy link
Contributor

sun-rui commented May 21, 2016

@HyukjinKwon, maybe we can merge this first. I will probably have a try later.
cc @shivaram

@shivaram
Copy link
Contributor

@HyukjinKwon - Thanks a lot for updating the documentation and for working on #13165 - I think there are a number of R users who use Windows and having SparkR work on Windows (at least for the purposes of trying it out) is useful for the Spark project.

That said it has been hard to find people who have the time / resources to test SparkR on Windows machines. I wrote the initial instructions for building in Windows but a bunch of stuff has changed since then.

I think this documentation change LGTM and we can come back and update it if we find a problem.

Merging this to master, branch-2.0

@HyukjinKwon
Copy link
Member Author

@shivaram Thank you so much.

asfgit pushed a commit that referenced this pull request May 24, 2016
## What changes were proposed in this pull request?

This PR adds the description for running unit tests in Windows.

## How was this patch tested?

On a bare machine (Window 7, 32bits), this was manually built and tested.

Author: hyukjinkwon <gurwls223@gmail.com>

Closes #13217 from HyukjinKwon/minor-r-doc.

(cherry picked from commit a8e97d1)
Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>
@asfgit asfgit closed this in a8e97d1 May 24, 2016
@HyukjinKwon HyukjinKwon deleted the minor-r-doc branch January 2, 2018 03:42
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.

6 participants