Skip to content

Automatically generate and commit site HTML - #699

Open
nchammas wants to merge 40 commits into
apache:asf-sitefrom
nchammas:automated-html
Open

nchammas wants to merge 40 commits into
apache:asf-sitefrom
nchammas:automated-html

Conversation

@nchammas

@nchammas nchammas commented Jul 12, 2026 •

Copy link
Copy Markdown
Contributor

Add workflows to automatically build the site and push the generated HTML. This eliminates the need for contributors to commit or review HTML changes.

What's Changing

Workflow changes:

  • html-build.yml: Add a new workflow that runs on PRs and simply generates the docs. This confirms the Jekyll build is not obviously broken.
  • html-push.yml: Add a new workflow that runs on new commits to asf-site. It generates the docs and pushes the generated HTML automatically as a new commit to asf-site.
  • doc_gen.yml: Delete this workflow since it is subsumed by the new workflows.

Website source changes:

  • Stabilize the sitemap so it's generated in a deterministic manner, eliminating repeated diff noise.

Repo documentation changes:

  • Update the README and release guide to account for the new workflows.
  • Unify the build instructions so there are only two places to look: committers.md and the main repo README.
  • Add important note about content/ so its critical role is documented clearly for everyone.

This is a second attempt at #697 (which was reverted in #698).

This PR needs to be paired with apache/spark#59071.

Why it's Changing

The current workflow creates a lot of friction. Every website source change generates corresponding HTML output changes. Both need to be committed and part of the PR. This makes the diff messy and difficult to review, especially if the touched source affects multiple pages.

Generating and committing the HTML is also just mechanical grunt work that can and should simply be automated.

Future Work

There are two main improvements we can further make to the workflow here.

  1. Split source from HTML output into two separate branches: source on master, HTML on asf-site.
  2. Automatically publish a staging site for each PR.

These improvements are orthogonal to the work in this PR. We can tackle them next.

@nchammas

Copy link
Copy Markdown
Contributor Author

cc @cloud-fan @holdenk

If you'd like to setup a staging site to test this out, I'm happy to help with that.

Otherwise, if/when we merge this, let's confirm the deployed production site is still working as expected so we can revert quickly if necessary.

@nchammas
nchammas marked this pull request as draft July 16, 2026 14:20
@nchammas

Copy link
Copy Markdown
Contributor Author

Switching PR to draft until all the current releases are out, per the earlier discussion.

@nchammas

Copy link
Copy Markdown
Contributor Author

To be clear this is an explicit but temporary veto on changing the website build until after 4.2.0/4.1.3/4.0.4/3.5.9 are all released.

It looks like these releases are all complete. @holdenk - Am I OK to mark this PR as ready for review?

To summarize the next steps given what we've discussed so far:

  1. Review and merge this PR. It will eliminate the need for anyone to generate or commit HTML in their PRs.
  2. The committer who merges this PR in should check the site immediately after the build runs to confirm the site deployed correctly and no revert is needed. (I believe that what broke the site in Add workflow to automatically generate and commit site HTML #697 was not related to the generated HTML, but I cannot be sure since we don't have a staging site!)
  3. In the future, we should explore the following further improvements to the website workflow, both of which are valuable:
    • Setup a dedicated staging site where we can test sensitive changes. If possible, setup automatic staging sites that spin up for each PR.
    • Split the Markdown source and HTML output into separate branches.

Does this sound good to everyone?

@holdenk

holdenk commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

I think we're good we can remove my soft -1 now that the patch releases are done. I do want @celestehorgan to take a look since she ran into some difficulty when she went to update the website.

@nchammas
nchammas marked this pull request as ready for review August 5, 2026 19:37

@celestehorgan celestehorgan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some non-blocking comments, but thank you for doing this work!

Comment thread .github/actions/build-html/action.yml
Comment thread README.md
@nchammas

Copy link
Copy Markdown
Contributor Author

I'm ready to move forward with this. And since I can commit to this repo now, I can monitor the deployment and immediately revert this change if anything goes wrong.

  1. Are there any outstanding concerns before I move forward?
  2. I noticed that on this repo we sometimes use GitHub's merge button rather than the merge script. Is it OK if I use GitHub's "Squash and merge"? Do we need the merge script at all on this repo?

@nchammas

Copy link
Copy Markdown
Contributor Author

@cloud-fan @holdenk @pan3793 - I'd like the go-ahead from at least one of you before I merge this PR. Do you have any outstanding concerns?

@nchammas

Copy link
Copy Markdown
Contributor Author

We're coming up on the 4.3.0 release and I don't want to make @HeartSaVioR's life more difficult, so I don't mind holding off again on moving this forward if he asks for that.

At the same time, I really do want to get this done so we can all move on from futzing with generated HTML output. I can just merge this in using my new powers as committer, but as this is a shared workflow for everyone I would prefer to have some formal buy-in on this PR before I do that.

@cloud-fan @holdenk @pan3793 - If you could take another look at this PR I would really appreciate it!

since we want site/ to be purely derivative (as opposed to containing unique source)
it was meant as a convenience but is too risky given how dev scripts on main spark repo are currently written

@cloud-fan cloud-fan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review summary

PR tags: behavior-change

I found two non-blocking inconsistencies in the new contribution guidance and one validation-policy question. The README needs to cover all website source types, and the pull-request template needs to preserve the separately built site/docs/<version> release exception. Please also confirm whether accepting non-fatal Jekyll stderr was an intentional change from the deleted workflow. The automated build-and-publish structure otherwise matches the stated scope, and I found no P0 or P1 blocker, but the unresolved warning-policy choice prevents an unconditional readiness conclusion.

Findings

2 total: 0 P0, 0 P1, 2 P2, 0 P3.

Non-blocking (P2)

  • Broaden the source-file guidance — README.md:9 — see inline.
  • Preserve the release-docs exception — .github/pull_request_template.md:2 — see inline.

Decision challenges

Confirm the Jekyll warning policy

The deleted doc_gen workflow captured Jekyll stderr and failed on messages outside its allowlist, while the shared replacement action only checks the bundle exec jekyll build exit status. Was dropping that warning gate intentional? If not, please preserve equivalent diagnostic filtering in the shared action so both the PR and publication workflows retain it.

Comment thread README.md Outdated

## Contributing

To contribute changes, build and test the site locally, then submit a pull request with your changes. You only need to commit changes to the Markdown source. A [GitHub Actions workflow](.github/workflows/html-push.yml) will generate the corresponding HTML under `site/` and push it for you.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking (P2): js/downloads.js, site/static/versions.json, and other non-Markdown files are website source too, and the release guide asks contributors to commit them. Saying only Markdown needs to be committed can therefore produce incomplete PRs. Please say to commit all source changes while excluding only ordinary Jekyll-generated site output.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These non-Markdown files are already addressed in the dedicated release guide. The typical PR does not need to worry about them. I'll tweak the wording here without repeating details that IMO are best left to the release guide.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, the current wording works for me: it says to commit source files, with Markdown only as the typical case, and leaves release-specific details in the release guide.

Comment thread .github/pull_request_template.md Outdated
@@ -0,0 +1,5 @@
<!--
Include your source changes, but not the generated HTML. A GitHub workflow will automatically generate and push the HTML under `site/` in a follow-up commit if necessary.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking (P2): This blanket instruction also covers release PRs, but the release process explicitly commits separately built docs under site/docs/<version>; Jekyll preserves that tree and cannot recreate it. Please exclude only ordinary Jekyll-generated site HTML and call out the versioned release-doc exception.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resolved in the current wording: the template excludes only Jekyll-generated HTML and sends release PRs to the guide, which explicitly commits site/docs/ and site/docs/latest.

@cloud-fan

Copy link
Copy Markdown
Contributor

I don't see a merge-method rule established by this PR's code or description. Please use the spark-website maintainer convention for the merge itself; someone with authority over that repository policy should confirm whether squash-and-merge is acceptable.

@cloud-fan

Copy link
Copy Markdown
Contributor

I reviewed the current changes and have two scoped items: please clarify whether dropping the prior Jekyll stderr/warning gate is intentional, and update the README to say contributors should commit all source changes while omitting only generated site output.

@nchammas

Copy link
Copy Markdown
Contributor Author

The deleted doc_gen workflow captured Jekyll stderr and failed on messages outside its allowlist, while the shared replacement action only checks the bundle exec jekyll build exit status. Was dropping that warning gate intentional?

Yes, intentional. Jekyll reports its exit status correctly, and failing on messages sent on stderr isn't something we do anywhere else. It's not clear to me why this was done in the first place, and the discussion on #316 doesn't add any clarity either.

I don't see a merge-method rule established by this PR's code or description. Please use the spark-website maintainer convention for the merge itself; someone with authority over that repository policy should confirm whether squash-and-merge is acceptable.

Hmm, I'm not sure what prompted this comment. But we discussed this on the dev list and I made the corresponding change in #707. All spark-website PRs are henceforth merged only using GitHub's "Squash and merge" button.

@cloud-fan cloud-fan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review summary

The two documentation inconsistencies from the prior review are resolved in the current README and pull-request template, and the author clarified that relying on Jekyll's exit status is intentional. This round found one late-caught blocker: both replacement jobs dropped the predecessor's pre-checkout disk cleanup, leaving the approximately 23 GiB working tree able to exhaust hosted-runner space before validation or publication starts.

Findings

1 total: 0 P0, 1 P1, 0 P2, 0 P3.

Blocking (P1)

  • Restore pre-checkout disk reclamation — .github/workflows/html-build.yml:13 — see inline.

Re-review status

Prior AI findings: 2 addressed, 0 still present; additional unresolved findings in this review: 1.

New attribution: 0 newly introduced, 1 late catch, 0 previously raised, 0 unattributed.

Remaining prior AI findings

No prior AI findings remain.

name: Build HTML
runs-on: ubuntu-24.04
steps:
- name: Checkout Spark Website repository

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking (P1): Both replacement jobs now make checkout their first substantive step, but the deleted job reclaimed large preinstalled packages before checking out this roughly 23 GiB tree. On the selected hosted runner, checkout can exhaust the available disk before either the PR build or the post-merge HTML job reaches the shared action. Please restore sufficient disk preparation ahead of checkout in both workflows, or use an equivalent checkout/storage design with demonstrated capacity.

Verification:

  • Inspection: Verify that both affected job definitions place equivalent sufficient disk reclamation ahead of every checkout path.
  • Behavior: Verify representative pull-request and non-bot asf-site push executions can complete checkout and reach the shared HTML build on the selected hosted runner.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The disk cleanup is not necessary. You can see that the build has been running on this PR just fine, and that includes a full checkout of the repo, including site/.

The reason it runs fine is because there is ~90 GB of free space on a 150 GB disk for the default public repo runners. This is different from the 14 GB number shared in the docs.

The nuance here is that the 150 GB disk is an official GitHub commitment for large runners only, not the regular runners we use:

We have no plans to reduce the 150 GB disk on 4-core runners back to 75 GB in the near future. That said, we can't guarantee it will stay that way forever.

So this is working now and will likely work fine for the foreseeable future. That said, if you really want to future-proof this, we can either use a large runner (which I think needs ASF approval) or we can reintroduce some form of disk cleanup step. I personally don't think either is necessary for now, but I'm fine with any approach: a) do nothing; b) use large runner; c) create new composite action for disk cleanup and use it.

@cloud-fan - What would you like to do?

@cloud-fan

Copy link
Copy Markdown
Contributor

Thanks, that answers both questions. I see that dropping the stderr gate is intentional, and that #707 established squash-and-merge for this repository.

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