Set full source_file path for default configuration
#4379
Conversation
| @@ -68,7 +70,7 @@ def load_yaml_config(version): | |||
| config = BuildConfig( | |||
| env_config=env_config, | |||
| raw_config={}, | |||
| source_file='empty', | |||
| source_file=path.join(checkout_path, 'empty'), | |||
There was a problem hiding this comment.
In the future, we want to manage this in a less hacky way (I mean the empty file)
There was a problem hiding this comment.
It seems like we'd want to just pass None rather than 'empty' but possibly that is a larger refactor.
| @@ -1153,7 +1153,10 @@ def assertArgsStartsWith(self, args, function_mock): | |||
| if arg is not mock.ANY: | |||
| self.assertTrue(arg_mock.startswith(arg)) | |||
|
|
|||
| def test_install_core_requirements_sphinx(self): | |||
| @patch('readthedocs.projects.models.Project.checkout_path') | |||
There was a problem hiding this comment.
This shouldn't be necessary if we remove
I didn't do it here because the changes would be bigger.
|
To help validate this change and improve documentation it would help to have a list of configuration parameters with: where they can be provided (web admin interface vs. configuration yaml file), what the default is when not provided either place, and what takes precedence when provided both places. |
Do you have an example for that? |
|
Yes example configuration file in root repository folder setting requirements_file https://gerrit.onap.org/r/gitweb?p=doc.git;a=blob;f=readthedocs.yml;h=6e6b9af07e6d523a60342e76bf181288bf826c63;hb=HEAD |
|
@rb2745 thank you! |
|
@rb2745 we take precedence to the settings in the configuration file, if a setting is not given, it defaults to the ones from the web interface. This will change with the v2 of the configuration file and be appropriated documented. |
Added as readthedocs in broken as per readthedocs/readthedocs.org#4379 Added: readthedocs.yml
| @@ -68,7 +70,7 @@ def load_yaml_config(version): | |||
| config = BuildConfig( | |||
| env_config=env_config, | |||
| raw_config={}, | |||
| source_file='empty', | |||
| source_file=path.join(checkout_path, 'empty'), | |||
There was a problem hiding this comment.
It seems like we'd want to just pass None rather than 'empty' but possibly that is a larger refactor.
|
Re labelled again as hotfix since it's already in production. We will have a release comming out soon, though. We re-trigger a build of freqgen and it passed: https://readthedocs.org/projects/freqgen/builds/7500721/ |
This was introduced in #4298. I can't replicate this locally (I think this is something to do with the builds being executed in another server). But debugging this I think I found the problem.
Before #4298 the
requirements_filewasn't validated, but now it is. For the validation, we use the dir name of thesource_file. This was''(empty/cwd). To fix this I'm passing the fullsource_fileas the checkout path of the build.This only affects people using the web interface to set the requirements file.
As workaround users can use a configuration file to set the
requirements_filesettingFix #4378