Skip to content

Build: run cat readthedocs.yaml even when there is a config error #11097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
humitos opened this issue Feb 6, 2024 · 4 comments · Fixed by #11175
Closed

Build: run cat readthedocs.yaml even when there is a config error #11097

humitos opened this issue Feb 6, 2024 · 4 comments · Fixed by #11175
Assignees
@humitos
Labels
Improvement Minor improvement to code

Comments

@humitos
Copy link
Member

humitos commented Feb 6, 2024 •
edited
Loading

I just realized that we first validate the readthedocs.yaml file and then run cat readthedocs.yaml to show its content. What happens? If there is a syntax error or a missing required value or anything, we don't run the cat command. This makes hard to debug and understand what's going on.

We should first run the cat command and after that validate the file. This will help a lot with UX.

Example: https://beta.readthedocs.org/projects/test-builds/builds/23364407/
@humitos humitos added the Improvement Minor improvement to code label Feb 6, 2024
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Feb 6, 2024
@humitos
Copy link
Member Author

humitos commented Feb 28, 2024 •
edited
Loading

Related to

@humitos
Copy link
Member Author

humitos commented Feb 29, 2024

This issue would have been being helpful on this case #11173 where the user had two YAML file in the repository and Read the Docs was picking the incorrect one.

@humitos
Copy link
Member Author

humitos commented Feb 29, 2024

I took a look at this and it seems we need to split the readthedocs.config.config.load() function into two:

  1. find the YAML file to load
  2. load the YAML file and validate it

Moving out 1) will allow us to know what's the file we will be loading and show it to the user before loading and validating.

@humitos humitos self-assigned this Feb 29, 2024
@humitos humitos moved this from Planned to Needs review in 📍Roadmap Feb 29, 2024
humitos added a commit that referenced this issue Feb 29, 2024
@humitos
Build: show the YAML config file before validating it
d6f367b 
This moves the `cat readthedocs.yaml` command _before_ validating the YAML file.
This allows the user to inspect the configuration file in case it fails due to a
validation error.

Closes #11097
@humitos humitos mentioned this issue Feb 29, 2024
Merged
@agjohnson agjohnson mentioned this issue Feb 29, 2024
Open
@agjohnson
Copy link
Contributor

I think this an okay first step to take, but discussed next steps on this particular command more in #7005.

humitos added a commit that referenced this issue Mar 4, 2024
@humitos
Build: show the YAML config file before validating it (#11175)
a49f111 
* Build: show the YAML config file before validating it

This moves the `cat readthedocs.yaml` command _before_ validating the YAML file.
This allows the user to inspect the configuration file in case it fails due to a
validation error.

Closes #11097

* Use a better name for the variable

* Omit check for `cat readthedocs.yaml` for now

We can't perform this check on the "build environment" because it happens in the
"VCS environment" now and we are not mocking it.

We should find a way to check these commands on the "VCS environment".
@github-project-automation github-project-automation bot moved this from Needs review to Done in 📍Roadmap Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@humitos humitos

Labels
Improvement Minor improvement to code
Projects
📍Roadmap
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Build: show the YAML config file before validating it readthedocs/readthedocs.org
2 participants
@humitos @agjohnson