[SPARK-30879][DOCS] Refine workflow for building docs by nchammas · Pull Request #27534 · apache/spark

nchammas · 2020-02-10T23:05:30Z

What changes were proposed in this pull request?

This PR makes the following refinements to the workflow for building docs:

Install Python and Ruby consistently using pyenv and rbenv across both the docs README and the release Dockerfile.
Pin the Python and Ruby versions we use.
Pin all direct Python and Ruby dependency versions.
Eliminate any use of sudo pip, which the Python community discourages, or sudo gem.

Why are the changes needed?

This PR should increase the consistency and reproducibility of the doc-building process by managing Python and Ruby in a more consistent way, and by eliminating unused or outdated code.

Here's a possible example of an issue building the docs that would be addressed by the changes in this PR: #27459 (comment)

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manual tests:

I was able to build the Docker image successfully, minus the final part about RUN useradd.
- I am unable to run do-release-docker.sh because I am not a committer and don't have the required GPG key.
I built the docs locally and viewed them in the browser.

I think I need a committer to more fully test out these changes.

…0731-building-docs

HyukjinKwon · 2020-02-11T00:22:56Z

@@ -20,9 +20,9 @@
 # Includes:


@dongjoon-hyun and @wangyum might better to review.

SparkQA · 2020-02-11T01:44:45Z

Test build #118187 has finished for PR 27534 at commit bc15b86.

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

nchammas · 2020-02-13T07:50:09Z

Should I break up this PR to make it easier to review? (Edit: Done.)

e.g.

Makefile trim (split to [SPARK-30880][DOCS] Delete Sphinx Makefile cruft #27625)
pages -> nav (split to [SPARK-30731] Update deprecated Mkdocs option #27626)
pyenv and rbenv + dependency pinning

Asking just in case the silence from other reviewers is because this PR is too big.

If y'all just haven't gotten around to taking a look, no worries. I'm being proactive.

…0731-building-docs

nchammas · 2020-02-18T19:39:08Z

I've split off the Makefile and Mkdocs changes and updated my previous comment with links to the new PRs.

SparkQA · 2020-02-18T21:46:12Z

Test build #118652 has finished for PR 27534 at commit 25fdaef.

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

nchammas · 2020-02-18T21:59:37Z

Retest this please.

nchammas · 2020-02-18T22:18:07Z

Is it possible for contributors to re-trigger the GitHub workflows for a certain commit? I'm not seeing how to do that.

srowen · 2020-02-26T15:01:16Z

FWIW, I don't think Affected Version really matters much for an improvement. You can set it to say it would be a valid improvement as of a version -- maybe it improves a thing that's only present from 2.4.0 onwards or something. But for most cases it probably 'affects' (could be applied to) lots of versions, and that isn't as interesting as in the case of a bug. Not required unless it's really important clarify, doesn't hurt in most cases either.

srowen · 2020-02-27T16:54:15Z

Just to be clear, any objections to merging?

nchammas · 2020-02-27T19:47:20Z

I don't think any committers have tried working with the proposed build instructions and/or Docker definition. Perhaps a committer could try things out to make sure it works for them?

srowen · 2020-03-05T23:30:35Z

Jenkins retest this please

SparkQA · 2020-03-06T02:40:26Z

Test build #119425 has finished for PR 27534 at commit 2b1b42f.

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

HyukjinKwon · 2020-03-06T02:41:52Z

-You need to have [Ruby](https://www.ruby-lang.org/en/documentation/installation/) and
-[Python](https://docs.python.org/2/using/unix.html#getting-and-installing-the-latest-version-of-python)
-installed. Also install the following libraries:
+You need to have Ruby 2 and Python 3 installed. A handy way to install and manage various versions of Ruby and Python is with [`rbenv`] and [`pyenv`].


Can we revert these doc changes in the documentation? Otherwise, I am good with other changes.

Or alternatively, we can have one section to describe use rbenv and pyenv separately .. that works to me too.

I moved them to their own sub-section. If we think it's distracting / not helpful, I can remove it.

SparkQA · 2020-03-06T21:23:16Z

Test build #119478 has finished for PR 27534 at commit ebba668.

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

HyukjinKwon · 2020-03-31T07:06:22Z

+ pandoc pandoc-citeproc libssl-dev libcurl4-openssl-dev libxml2-dev
+
+ENV PATH "$PATH:/root/.pyenv/bin:/root/.pyenv/shims"
+RUN curl -L https://github.com/pyenv/pyenv-installer/raw/dd3f7d0914c5b4a416ca71ffabdf2954f2021596/bin/pyenv-installer | bash


Okay, I suspect it was not tested due to the limitation described in the PR description:
Generating SQL API Markdown files. 20/03/31 06:41:42 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable Traceback (most recent call last): File "/opt/spark-rm/output/spark/sql/gen-sql-api-docs.py", line 21, in<module> from pyspark.java_gateway import launch_gateway File "/opt/spark-rm/output/spark/python/lib/pyspark.zip/pyspark/__init__.py", line 51, in<module> File "/opt/spark-rm/output/spark/python/lib/pyspark.zip/pyspark/context.py", line 22, in<module> ImportError: No module named threading log4j:WARN No appenders could be found for logger (org.apache.spark.util.ShutdownHookManager). log4j:WARN Please initialize the log4j system properly. log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
Seems the installed Python is weird. threading is the standard Python library that has existed from Python 2 to Python 3, but seems not existent with the Python installed here.
Let me revert this to make RC preparation easier.

Hmm, that's strange, because I was able to build enough of the Dockerfile to get past this point, and I did not hit this problem.
What I couldn't test was to run everything via do-release-docker.sh.

cloud-fan · 2020-03-31T07:11:41Z

We can manually test it by changing the ENTRYPOINT in Dockerfile to /bin/bash and check if the python and ruby are installed correctly.

The release script doesn't work anymore after this patch because we use a non-standard way to install python/ruby (download a bash script and run it):

The actual work is done by user spark-rm, which can't access /root/.pyenv
Even if we fix the permission issue, the installed python doesn't work well with the system libraries, and have errors like ImportError: No module named threading
It's wrong to append the PATH, and we should prepend it. Otherwise python still points to the system default which is 2.7.

HyukjinKwon · 2020-03-31T07:13:14Z

Reverted at 4d4c3e7.

dongjoon-hyun · 2020-03-31T09:00:58Z

BTW, are we using the release script in master branch?
We had better use the release script in branch-3.0 to avoid Apache Spark 2.4.2 situation.

cc @rxin since he is a release manager.

cloud-fan · 2020-03-31T09:09:12Z

Yea we should use the script in branch-3.0 so this doesn't block the RC. But we should still revert this PR as it doesn't work.

dongjoon-hyun · 2020-03-31T09:23:47Z

Sure~ Of course for reverting this PR.

HyukjinKwon · 2020-03-31T10:01:10Z

I think my wording made some miscommunication here:

Let me revert this to make RC preparation easier.

I simply meant future RCs :-).

nchammas · 2020-03-31T17:56:24Z

We can manually test it by changing the ENTRYPOINT in Dockerfile to /bin/bash and check if the python and ruby are installed correctly.
The release script doesn't work anymore after this patch because we use a non-standard way to install python/ruby (download a bash script and run it):
The actual work is done by user spark-rm, which can't access /root/.pyenv
Even if we fix the permission issue, the installed python doesn't work well with the system libraries, and have errors like ImportError: No module named threading
It's wrong to append the PATH, and we should prepend it. Otherwise python still points to the system default which is 2.7.

This is strange since I tested this successfully via docker build . from within the folder containing the Dockerfile.

What I couldn't do was test building the image via do-release-docker.sh, which I called out in the PR description and again in the comments.

I think the reason my test didn't see these issues is because the do-release-docker.sh script builds the image with some options that change the executing user from root.

run_silent "Building spark-rm image with tag $IMGTAG..." "docker-build.log" \
docker build --no-cache -t "spark-rm:$IMGTAG" --build-arg UID=$UID "$SELF/spark-rm"

I guess the moral of the story is that I couldn't test do-release-docker.sh directly, and my substitute test of docker build . was not a good test since it didn't account for the root vs. spark-rm user change.

I should have more carefully tried to duplicate the docker build command as it appeared in do-release-docker.sh, and insisted more forcefully that we not merge this PR in until a committer had had a chance to try out do-release-docker.sh directly.

Sorry.

nchammas · 2020-03-31T18:18:46Z

I can try taking another stab at this change for the future, now that I have a better idea of how to test it.

However, given that this change was met with some resistance to begin with, I won't try to fix it unless there is explicit committer interest.

cloud-fan · 2020-04-01T03:46:26Z

This PR does have a good point to fix the dependency versions so that the script is more robust. I'm happy to see a working version of it.

HyukjinKwon · 2020-04-01T04:18:52Z

We can pin the version for release related ones; however, I doubt if we should do that for others e.g., CI, documentation. It's being discussed at #27928

Split from apache#27534. ### What changes were proposed in this pull request? This PR updates a deprecated Mkdocs option to use the new name. ### Why are the changes needed? This change will prevent the docs from failing to build when we update to a version of Mkdocs that no longer supports the deprecated option. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? I built the docs locally and reviewed them in my browser. Closesapache#27626 from nchammas/SPARK-30731-mkdocs-dep-opt. Authored-by: Nicholas Chammas <nicholas.chammas@liveramp.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>

Split off from apache#27534. ### What changes were proposed in this pull request? This PR deletes some unused cruft from the Sphinx Makefiles. ### Why are the changes needed? To remove dead code and the associated maintenance burden. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Tested locally by building the Python docs and reviewing them in my browser. Closesapache#27625 from nchammas/SPARK-30731-makefile-cleanup. Lead-authored-by: Nicholas Chammas <nicholas.chammas@liveramp.com> Co-authored-by: Nicholas Chammas <nicholas.chammas@gmail.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>

### What changes were proposed in this pull request? This PR makes the following refinements to the workflow for building docs: * Install Python and Ruby consistently using pyenv and rbenv across both the docs README and the release Dockerfile. * Pin the Python and Ruby versions we use. * Pin all direct Python and Ruby dependency versions. * Eliminate any use of `sudo pip`, which the Python community discourages, or `sudo gem`. ### Why are the changes needed? This PR should increase the consistency and reproducibility of the doc-building process by managing Python and Ruby in a more consistent way, and by eliminating unused or outdated code. Here's a possible example of an issue building the docs that would be addressed by the changes in this PR: apache#27459 (comment) ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Manual tests: * I was able to build the Docker image successfully, minus the final part about `RUN useradd`. * I am unable to run `do-release-docker.sh` because I am not a committer and don't have the required GPG key. * I built the docs locally and viewed them in the browser. I think I need a committer to more fully test out these changes. Closesapache#27534 from nchammas/SPARK-30731-building-docs. Authored-by: Nicholas Chammas <nicholas.chammas@liveramp.com> Signed-off-by: Sean Owen <srowen@gmail.com>

nchammas added 12 commits February 4, 2020 21:16

trim down makefile cruft
f113896

pages is deprecated
be8a792

copy python api docs as part of make html
5275521

use version manager; pin dep versions
c6e7695

Merge branch 'master' ofhttps://github.com/apache/sparkinto SPARK-3…
4ca628e
…0731-building-docs

ignore pyenv and rbenv version files
a1b44f2

no cache for release docker image
5e06418

install python and ruby with pyenv and rbenv
8954672

revert a couple of unnecessary changes
b66c077

undo stuff related to copying built docs
f3cc9d4

spark/home -> spark/root
172fb22

unneeded makefile var
bc15b86

nchammas requested review from HyukjinKwon, dongjoon-hyun and wangyum February 10, 2020 23:53

HyukjinKwon reviewed Feb 11, 2020
View reviewed changes

Comment threadsql/mkdocs.yml Outdated

HyukjinKwon reviewed Feb 11, 2020
View reviewed changes

Comment threadpython/docs/Makefile

HyukjinKwon reviewed Feb 11, 2020
View reviewed changes

Comment threaddocs/README.md

HyukjinKwon reviewed Feb 11, 2020
View reviewed changes

Comment threaddocs/README.md Outdated

This was referenced Feb 18, 2020
[SPARK-30880][DOCS] Delete Sphinx Makefile cruft #27625
Closed
[SPARK-30731] Update deprecated Mkdocs option #27626
Closed

nchammas added 2 commits February 18, 2020 14:35

Merge branch 'master' ofhttps://github.com/apache/sparkinto SPARK-3…
25fdaef
…0731-building-docs

revert changes being tracked in other PRs
2b1b42f

dongjoon-hyun added the DOCUMENTATION label Feb 28, 2020

HyukjinKwon reviewed Mar 6, 2020
View reviewed changes

nchammas added 2 commits March 6, 2020 13:47

separate gem reqs from rbenv/pyenv suggestion
8d05f6d

state the obvious: install R
ebba668

srowen closed this in 7892f88Mar 7, 2020

nchammas deleted the SPARK-30731-building-docs branch March 9, 2020 18:44

nchammas mentioned this pull request Mar 14, 2020
[SPARK-31155] Remove pydocstyle tests #27912
Closed

HyukjinKwon reviewed Mar 31, 2020
View reviewed changes

cloud-fan mentioned this pull request Apr 1, 2020
[SPARK-31320] Fix release script for 3.0.0 #28088
Closed

nchammas mentioned this pull request Apr 1, 2020
[SPARK-31167][BUILD] Refactor how we track Python test/build dependencies #27928
Closed

Uh oh!

Conversation

nchammas commented Feb 10, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

Uh oh!

HyukjinKwonFeb 11, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Feb 11, 2020

Uh oh!

nchammas commented Feb 13, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nchammas commented Feb 18, 2020

Uh oh!

SparkQA commented Feb 18, 2020

Uh oh!

nchammas commented Feb 18, 2020

Uh oh!

nchammas commented Feb 18, 2020

Uh oh!

srowen commented Feb 26, 2020

Uh oh!

srowen commented Feb 27, 2020

Uh oh!

nchammas commented Feb 27, 2020

Uh oh!

srowen commented Mar 5, 2020

Uh oh!

SparkQA commented Mar 6, 2020

Uh oh!

HyukjinKwonMar 6, 2020

Choose a reason for hiding this comment

Uh oh!

HyukjinKwonMar 6, 2020

Choose a reason for hiding this comment

Uh oh!

nchammasMar 6, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 6, 2020

Uh oh!

HyukjinKwonMar 31, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nchammasMar 31, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Mar 31, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HyukjinKwon commented Mar 31, 2020

Uh oh!

dongjoon-hyun commented Mar 31, 2020

Uh oh!

cloud-fan commented Mar 31, 2020

Uh oh!

dongjoon-hyun commented Mar 31, 2020

Uh oh!

HyukjinKwon commented Mar 31, 2020

Uh oh!

nchammas commented Mar 31, 2020

Uh oh!

nchammas commented Mar 31, 2020

Uh oh!

cloud-fan commented Apr 1, 2020

Uh oh!

HyukjinKwon commented Apr 1, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

nchammas commented Feb 10, 2020•
edited
Loading

nchammas commented Feb 13, 2020•
edited
Loading

HyukjinKwonMar 31, 2020•
edited
Loading

cloud-fan commented Mar 31, 2020•
edited
Loading

HyukjinKwon commented Apr 1, 2020•
edited
Loading