Skip to content

RTD custom build#1096

Draft
atteggiani wants to merge 3 commits into
developmentfrom
davide/rtd_custom_build
Draft

RTD custom build#1096
atteggiani wants to merge 3 commits into
developmentfrom
davide/rtd_custom_build

Conversation

@atteggiani

@atteggiani atteggiani commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

Overview

Added custom build script to:

Note

The current RTD build fails because the readthedocs config is trying to fetch for the custom_build.sh file in main (after 5ebff2c), but that file is added in this PR and therefore cannot be found in main. This is the setup that will work in production.
A successful test using the script fetched from this PR head branch can be found here.

More information about implementation

Currently this PR sets a custom build script (custom_build.sh) to replace the default RTD build job.
This is necessary as env variables cannot be passed from a custom RTD job to another (separate RTD jobs are run with brand new environments and cannot inherit the enviroment from a previous custom job). This is the reason why the EDIT_URI env variable cannot be set in a custom job before build (for example post-checkout) and passed to the default RTD build step.

@atteggiani atteggiani requested a review from a team as a code owner December 4, 2025 04:29
@atteggiani atteggiani self-assigned this Dec 4, 2025
@github-actions

github-actions Bot commented Dec 4, 2025

Copy link
Copy Markdown
PR Preview
🚀 Preview of PR head commit e99afd0 deployed to https://docs.access-hive.org.au/pr-previews/1096
2026-07-03 16:47 AEST
Preview generated through the Deploy to GitHub Pages workflow run 28643610391.

@anton-seaice anton-seaice 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.

Is this change only required so that there isn't one dead link when making a new page in documentation ?

@atteggiani

atteggiani commented Dec 5, 2025

Copy link
Copy Markdown
Contributor Author

Is this change only required so that there isn't one dead link when making a new page in documentation ?

Yes, any new pages. In general we would also have a proper edit_uri set up "globally", without needing to specify multiple variations in the mkdocs.yml in case the filesystem of the repo is different. So we can use the EDIT_URI env variable in the mkdocs.yml for all websites and it would work.

@anton-seaice anton-seaice 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.

This is quite a complicated change for a very minor problem. This sort of customisation is difficult to understand and prone to breaking semi-regularly, so will add maintenance burden.

My first thought would be to not fix it. Is there consensus in @ACCESS-NRI/hivedocsteam to go ahead with this?

@KAUR1984

KAUR1984 commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

Thanks @atteggiani for proposing and working on this and @anton-seaice for your review. My understanding from the Hive Docs meeting yesterday was that fixing the pencil icon link is a nice improvement. But after looking at the details, it seems like the amount of custom code needed is pretty big, as already suggested by @anton-seaice, it increases ongoing maintenance load on us.

In this case, I would agree with the review and suggest not to introduce a complicated solution for a minor feature. A few additional reasons that I could think of:

  1. The benefit is very small, especially for PR previews.
  2. Simpler is easier to maintain and less likely to break.
  3. PR previews are temporary, and contributors don’t really need to use the pencil icon there.

Tip

Possible Solutions - We can still take a few lighter solutions:

  1. Explain this limitation clearly in the contribution guidelines.
  2. Ignore this link in the link checker to avoid CI failing.
  3. Disable the edit icon completely for PR previews (optional, given if it's trivial to implement and only a few lines of code).

Note

And honestly, for someone like me who isn’t the code owner, it looks like it would take quite a while to understand how all of this works before even attempting to fix it 😂.

@KAUR1984 KAUR1984 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.

Added my review as a comment here - #1096 (comment)

@atteggiani

Copy link
Copy Markdown
Contributor Author

This is quite a complicated change for a very minor problem. This sort of customisation is difficult to understand and prone to breaking semi-regularly, so will add maintenance burden.

My first thought would be to not fix it. Is there consensus in @ACCESS-NRI/hivedocsteam to go ahead with this?

I understand this adds some complication, but the problem is not minor, given that it would make the link-checker not usable . Why do you think the customisation is prone to breaking semi-regularly?

To me it seems like a pretty solid implementation. We can certainly improve commenting and explanations and possibly take some more steps to make it more foolproof, but I don't see too many sources of errors in the current implementation.

@anton-seaice

Copy link
Copy Markdown
Contributor

I understand this adds some complication, but the problem is not minor, given that it would make the link-checker not usable .

Link checking only fails when a new page is added right ?

Why do you think the customisation is prone to breaking semi-regularly?

It's reliant on many things from outside this repo and therefore not under out control. These are all prone to changing:

  • downloading executables from github
  • environment variables defined by others
  • read the docs not having a redesign
  • additional js from others etc

I'm not at all an expert on this stuff, maybe we need to seek some thoughts from other folk?

@atteggiani

atteggiani commented Dec 10, 2025

Copy link
Copy Markdown
Contributor Author
  1. The benefit is very small, especially for PR previews.

Unfortunately the benefit is not small, because without this the link-checker would not be usable consistently. There will always be cases in which it will fail and we don't have ways to control that, meaning each maintainer of the documentation would need to know why it fails and "ignore" the error (but only if it only fails for the know error). This is a massive development burden for everyone developing a documentation website.

  1. Simpler is easier to maintain and less likely to break.

The only actual additions are ~40 lines of bash code (of which ~10 are downloading yq and jq). I don't think that is a toll to maintain. As an example, the current GitHub Pages infrastructure is a way bigger burden to maintain.

  1. PR previews are temporary, and contributors don’t really need to use the pencil icon there.

I understand the pencil icon would not necessarily be needed for PRs, and in fact a solution would be removing that (it would still need a similar implementation but highly simplified).
However, this implementation also helps setting always the correct path for the edit pencil independently on the documentation website (Hive Docs, config docs, tools, etc.) because the path is taken directly from the build (which is the safer way to do it and avoids the burden of having to change it manually).

I think in general, despite the minor additions, this decreases the overall burden on development and makes it easier and quicker to setup another documentation website which uses the same infrastructure and "just works" as expected.

@anton-seaice

Copy link
Copy Markdown
Contributor

I will abstain from this - I don't think this or the edit pencil button are important enough to be a priority. Navigating the git website is easy enough to figure out how to edit the docs.

@atteggiani

atteggiani commented Dec 10, 2025

Copy link
Copy Markdown
Contributor Author

Thank you for your inputs @KAUR1984 and @anton-seaice.

I agree the "pencil icon" is not a vital feature, but is one that allows easier development and with the Hive Docs team we discussed about adding it to the Hive Docs and other documentation websites.
Also the link-checker is useful to easily spot non-working links, which makes development easier and produces a more reliable and usable product (websites that don't have broken links).

Also, this regards all documentation websites, for which in general the less manual changes required for specific websites the better.

To fully take advantage of all these features, a custom RTD build is needed, to allow setting of needed env variables so the mkdocs.yaml can be simplified and wouldn't need further manual changes, and the edit link can be set properly so the link-checker would work every time.

This custom RTD build is controlled by a single repo (this repo for the moment, but I'm thinking a separate dedicated repo might be created for that in the future) and this reduces a lot the development burden, because changes would only need to be applied one repo and would reflect in the build of all documentation websites.
This system can be improved and will likely be as we increase our usage of RTD and presence with documentation websites, but for the moment I think is a good starting point that works and allows the features described above (with minimal burden).

For the reasons above, please feel free to comment this PR with specific improvements for clarity and error-prone parts. I will also ask some more people to review, so we have a more comprehensive picture.

@atteggiani atteggiani force-pushed the davide/rtd_custom_build branch from 5ebff2c to 0913f4c Compare December 12, 2025 00:28

@CodeGat CodeGat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just some comments regarding the tooling that is being installed...

Comment thread .readthedocs/scripts/custom_build.sh
Comment on lines +23 to +29
gh_api_url="https://api.github.com/repos/$owner/$repo"
pr_info=$(curl -s "$gh_api_url/pulls/$READTHEDOCS_GIT_IDENTIFIER")
pr_head_owner=$("$JQ_EXE" -r '.head.user.login' <<< "$pr_info")
if [ "$owner" == "$pr_head_owner" ]; then # Not a fork
# Get the branch name
branch_name=$("$JQ_EXE" -r '.head.ref' <<< "$pr_info")
fi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Similarly, if this is running on GitHub Runners, gh is installed, and may be a bit easier to work with than calling the api via curl

@atteggiani atteggiani Dec 15, 2025

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.

Same as above, unfortunately I don't think gh is available out of the box within AWS instances.

@atteggiani atteggiani Dec 15, 2025

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.

Here the choice might be whether to download gh (and using jq within it), or download jq and use curl ... for API queries (as it is done currently).
Not sure if there's any preferred approach as we would always need to download something.
What do you think?

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.

Also, as a general approach, do you think it might make more sense to pin the versions of the external software we download instead of using latest?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, I think if you can get away with using gh with it's inbuilt jq that would be cleaner. However note that you would need to find a way to pass a suitable GitHub PAT to gh to enable use of it.

And yes, pin versions of external software. It means one day when any of the tools has a breaking change, you won't need to spend time figuring out why the workflow broke!

@atteggiani atteggiani Dec 15, 2025

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.

Yes, I remember now I chose to use curl over gh so I wouldn't need a PAT. The problem is that secrets cannot be set in RTD to be used by PRs. Secrets can only be used by "internal" builds, otherwise they will need to be set as plain env variables (i.e., they will be visible to anyone that has access to the RTD project settings/build, and that's definitely not what we want).

Therefore, I think I'll keep the use of jq and yq as it is, but update the script to pin specific versions.

Comment thread .readthedocs/scripts/custom_build.sh
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.

Create pencil icon on each page on Hive Docs

4 participants