[SPARK-38210][DOCS] Improve documentation generation README by khalidmammadov · Pull Request #35516 · apache/spark

khalidmammadov · 2022-02-14T22:43:34Z

What changes were proposed in this pull request?

Current instructions in README file is not complete and not sufficient to complete site build for testing and validation.
After number of trial and errors I have managed to build it. In the process I had to install number of additional packages.
This PR purposes improvements to the documentation to avoid spending similar efforts for contributors.

Why are the changes needed?

Improve Spark documentation generation procedure

Does this PR introduce any user-facing change?

No

How was this patch tested?

I have started a docker container:
docker run --name spark_doc_build_new -p 4000:4000 -it spark_doc_build_image
and installed everything as per below

apt-get update
apt-get -y install  git nano
apt-get -y install  curl
apt-get -y install  ruby-full
apt-get -y install  python3 pip
apt-get -y install  gnupg

echo "deb https://cloud.r-project.org/bin/linux/ubuntu focal-cran40/" >> /etc/apt/sources.list
apt-key adv --keyserver keyserver.ubuntu.com --recv-key '95C0FAF38DB3CCAD0C080A7BDC78B2DDEABC47B7'
apt-get update

apt-get -y install  r-base
apt-get -y install  pandoc libxml2-dev
apt-get -y install  libcurl4-openssl-dev
apt-get -y install  libssl-dev
apt-get -y install  libfontconfig1-dev libharfbuzz-dev   libfribidi-dev libfreetype6-dev libpng-dev libtiff5-dev libjpeg-dev

Rscript -e 'install.packages(c("curl", "knitr", "devtools", "testthat", "rmarkdown", "markdown", "e1071"), repos="https://cloud.r-project.org/")'
Rscript -e 'devtools::install_version("roxygen2", version = "7.1.2", repos="https://cloud.r-project.org/")'
Rscript -e "devtools::install_version('pkgdown', version='2.0.1', repos='https://cloud.r-project.org')"
Rscript -e "devtools::install_version('preferably', version='0.4', repos='https://cloud.r-project.org')"

echo 'deb http://security.debian.org/debian-security stretch/updates main' >> /etc/apt/sources.list
apt-get update
apt-get -y install  openjdk-8-jdk
apt-get -y install  scala

git clone https://github.com/apache/spark.git
cd spark/doc

gem install bundler
bundle install
bundle exec jekyll build

and checked via jekyll serve from host
bundle exec jekyll serve --host 0.0.0.0

khalidmammadov · 2022-02-14T23:09:39Z

I have also made this Dockerfile to make the process even easier. Would that be valuable to add to the repo?
https://github.com/khalidmammadov/spark/pull/1/files

HyukjinKwon · 2022-02-15T01:41:07Z

docs/README.md


 ```sh
-$ sudo Rscript -e 'install.packages(c("knitr", "devtools", "testthat", "rmarkdown"), repos="https://cloud.r-project.org/")'
+$ sudo Rscript -e 'install.packages(c("curl", "knitr", "devtools", "testthat", "rmarkdown", "markdown", "e1071"), repos="https://cloud.r-project.org/")'


Hm, rmarkdown depends on markdown IIRC. rmarkdown falls back to markdown. Was this required in your env?

cc @huaxingao FYI who faced a similar problem before IIRC ..

I think I finally managed to understand what's going on...
So, I'm using this Docker for the build. And tested with and without markdown package and it fails without and I couldn't understand how it succeeds in the build and test CI phase. So, apparently it's installed on the base image (and others I am adding here) from @dongjoon-hyun 's Docker image (BTW, where is the source of this Dockerfile kept?). So, some packages are "reinstalled" during Build and test and some not hence the confusion.
Additionally, I tested building "without" rmarkdown and it succeeds.

I had the same problem: I tested with and without markdown package and it failed without.

I re-tested that number of times in docker containers and it always fails if package is not installed. So, yes, in short markdown is required package.

HyukjinKwon · 2022-02-15T01:41:52Z

docs/README.md

 whichever version of Spark you currently have checked out of revision control.

-## Prerequisites
+## Building documentation


HyukjinKwon · 2022-02-15T01:43:17Z

docs/README.md

+If you'd like to generate R API documentation, you'll need to install these packages and libraries:
+
+```sh
+$ sudo apt install libssl-dev libcurl4-openssl-dev pandoc libfontconfig1-dev libharfbuzz-dev \


Hm, I think we should better make it independent from the OS

Do you have any suggestion? I can only suggest adding a Dockerfile similar to this one to build and test the changes or omit these installs as they are for linux? In the last case it makes again not complete and one needs to figure it out what to install every time.

srowen · 2022-02-15T14:51:11Z

docs/README.md


-## Prerequisites
+## Building documentation
+There are two ways to build Spark documentation, complete and partial. Complete will build a site similar


-> "similar to the main documentation site at ..."
Start a new sentence like "with all APIs documented. Partial ..."
I think this could be clarified: "Partial documentation builds, for a specific language or API, are also possible"

Newline after section heading, like others

srowen · 2022-02-15T14:51:21Z

docs/README.md


-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)
+For complete documentation all below tools must be installed **including Optionals**.


below tools -> tools below

srowen · 2022-02-15T14:51:43Z

docs/README.md

-[Python](https://docs.python.org/2/using/unix.html#getting-and-installing-the-latest-version-of-python)
+For complete documentation all below tools must be installed **including Optionals**.
+
+You need to have JDK, Scala, [Ruby](https://www.ruby-lang.org/en/documentation/installation/) and


JDK -> the JDK

srowen · 2022-02-15T15:02:19Z

docs/README.md

 ```

-## API Docs (Scaladoc, Javadoc, Sphinx, roxygen2, MkDocs)
+You can optionally skip API build (for partial build) as it takes time


This needs a rewrite: "To create a partial build without API docs (which can take a long time), use SKIP_API=1:"
But then I thought partial builds were just the API docs? this addition is confusing

AmplabJenkins · 2022-02-16T08:14:11Z

Can one of the admins verify this patch?

srowen

I find a lot of this change a bit confusing, not sure it is helping docs

srowen · 2022-02-20T19:35:49Z

docs/README.md


-## Prerequisites
+## Building documentation
+There are two ways to build Spark documentation, complete and partial. Complete will build a site similar


Newline after section heading, like others

srowen · 2022-02-20T19:36:20Z

docs/README.md

 ```

-## API Docs (Scaladoc, Javadoc, Sphinx, roxygen2, MkDocs)
+## Generating individual API Docs (Scaladoc, Javadoc, Sphinx, roxygen2, MkDocs)


APIs are "Scala", "Java", "Python", "R". roxygen2, mkdocs, sphinx are not APIs

Also, I'm confused, weren't the sections above already about generating individual API docs?

srowen · 2022-02-20T19:56:36Z

docs/README.md


-NOTE: To skip the step of building and copying over the Scala, Java, Python, R and SQL API docs, run `SKIP_API=1
-bundle exec jekyll build`. In addition, `SKIP_SCALADOC=1`, `SKIP_PYTHONDOC=1`, `SKIP_RDOC=1` and `SKIP_SQLDOC=1` can be used
+NOTE: To skip the step of building and copying over the Scala, Java, Python, R and SQL API docs, see below example. 


"see the example below"

Improve documentation generation README

d53ee8b

github-actions bot added the DOCS label Feb 14, 2022

HyukjinKwon reviewed Feb 15, 2022

View reviewed changes

srowen requested changes Feb 15, 2022

View reviewed changes

Fix review comments

b4145e7

khalidmammadov mentioned this pull request Feb 20, 2022

[SPARK-38261][INFRA] Add missing R packages from base image #35583

Closed

khalidmammadov requested review from HyukjinKwon and srowen February 20, 2022 18:02

srowen reviewed Feb 20, 2022

View reviewed changes

srowen closed this Mar 24, 2022

Conversation

khalidmammadov commented Feb 14, 2022

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!

khalidmammadov commented Feb 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Feb 16, 2022

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants