You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "nchammas (via GitHub)" <gi...@apache.org> on 2024/01/08 15:01:23 UTC

[PR] Ensure that docs build successfully with SKIP_API=1 [spark]

nchammas opened a new pull request, #44627:
URL: https://github.com/apache/spark/pull/44627

   ### What changes were proposed in this pull request?
   
   This PR tweaks the docs build so that the general docs are first built with `SKIP_API=1` to ensure that the docs build works without any language being built beforehand.
   
   ### Why are the changes needed?
   
   [Committers expect][1] docs to build with `SKIP_API=1` on a fresh checkout. Yet, our CI build does not ensure this. This PR corrects this gap.
   
   [1]: https://github.com/apache/spark/pull/44393/files#r1444169083
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   Via test commits against this PR.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on code in PR #44627:
URL: https://github.com/apache/spark/pull/44627#discussion_r1445552884


##########
.github/workflows/build_and_test.yml:
##########
@@ -761,6 +761,9 @@ jobs:
       run: ./dev/lint-r
     - name: Run documentation build
       run: |
+        # Build docs first with SKIP_API to ensure they are buildable without requiring any
+        # language docs to be built beforehand.
+        cd docs; SKIP_API=1 bundle exec jekyll build; cd ..
         if [ -f "./dev/is-changed.py" ]; then
           # Skip PySpark and SparkR docs while keeping Scala/Java/SQL docs
           pyspark_modules=`cd dev && python3.9 -c "import sparktestsupport.modules as m; print(','.join(m.name for m in m.all_modules if m.name.startswith('pyspark')))"`

Review Comment:
   Hmm, could you elaborate a little bit on this? How would this prevent [yesterday's scenario][1]?
   
   [1]: https://github.com/apache/spark/pull/44393#discussion_r1444169083



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #44627: [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1
URL: https://github.com/apache/spark/pull/44627


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #44627:
URL: https://github.com/apache/spark/pull/44627#discussion_r1446769858


##########
.github/workflows/build_and_test.yml:
##########
@@ -761,6 +761,9 @@ jobs:
       run: ./dev/lint-r
     - name: Run documentation build
       run: |
+        # Build docs first with SKIP_API to ensure they are buildable without requiring any
+        # language docs to be built beforehand.
+        cd docs; SKIP_API=1 bundle exec jekyll build; cd ..
         if [ -f "./dev/is-changed.py" ]; then
           # Skip PySpark and SparkR docs while keeping Scala/Java/SQL docs
           pyspark_modules=`cd dev && python3.9 -c "import sparktestsupport.modules as m; print(','.join(m.name for m in m.all_modules if m.name.startswith('pyspark')))"`

Review Comment:
   Actually let's just merge this. 5 secs won't affect the elapsed time much anyway.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #44627:
URL: https://github.com/apache/spark/pull/44627#discussion_r1445537517


##########
.github/workflows/build_and_test.yml:
##########
@@ -761,6 +761,9 @@ jobs:
       run: ./dev/lint-r
     - name: Run documentation build
       run: |
+        # Build docs first with SKIP_API to ensure they are buildable without requiring any
+        # language docs to be built beforehand.
+        cd docs; SKIP_API=1 bundle exec jekyll build; cd ..
         if [ -f "./dev/is-changed.py" ]; then
           # Skip PySpark and SparkR docs while keeping Scala/Java/SQL docs
           pyspark_modules=`cd dev && python3.9 -c "import sparktestsupport.modules as m; print(','.join(m.name for m in m.all_modules if m.name.startswith('pyspark')))"`

Review Comment:
   or wonder if we should set `SKIP_SCALADOC=1` together with `if [ `./dev/is-changed.py -m ...` condition. That might be better.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44627:
URL: https://github.com/apache/spark/pull/44627#issuecomment-1881569480

   cc @HyukjinKwon 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #44627:
URL: https://github.com/apache/spark/pull/44627#issuecomment-1884019267

   Merged to master.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #44627:
URL: https://github.com/apache/spark/pull/44627#discussion_r1445498806


##########
.github/workflows/build_and_test.yml:
##########
@@ -761,6 +761,9 @@ jobs:
       run: ./dev/lint-r
     - name: Run documentation build
       run: |
+        # Build docs first with SKIP_API to ensure they are buildable without requiring any
+        # language docs to be built beforehand.
+        cd docs; SKIP_API=1 bundle exec jekyll build; cd ..

Review Comment:
   LGTM but just to confirm this doesn't take a lot of time right?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [MINOR][INFRA] Ensure that docs build successfully with SKIP_API=1 [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on code in PR #44627:
URL: https://github.com/apache/spark/pull/44627#discussion_r1445548531


##########
.github/workflows/build_and_test.yml:
##########
@@ -761,6 +761,9 @@ jobs:
       run: ./dev/lint-r
     - name: Run documentation build
       run: |
+        # Build docs first with SKIP_API to ensure they are buildable without requiring any
+        # language docs to be built beforehand.
+        cd docs; SKIP_API=1 bundle exec jekyll build; cd ..

Review Comment:
   Correct. This adds around 5 seconds to the "Run documentation build" step.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org