-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Implement mkdocs key from v2 config #4486
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.
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
Changes from 11 commits
c1d791e
5918ce0
019fc27
8b077e9
050c974
c469d64
ce16fe7
9caace0
d9d2f25
b0d23a6
3962715
1aa8126
ca60dca
96f6241
a03da2b
357f034
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,7 +138,7 @@ def __init__(self, env_config, raw_config, source_file, source_position): | |
def error(self, key, message, code): | ||
"""Raise an error related to ``key``.""" | ||
source = '{file} [{pos}]'.format( | ||
file=self.source_file, | ||
file=os.path.relpath(self.source_file, self.base_path), | ||
pos=self.source_position, | ||
) | ||
error_message = '{source}: {message}'.format( | ||
|
@@ -533,6 +533,13 @@ def sphinx(self): | |
fail_on_warning=False, | ||
) | ||
|
||
@property | ||
def mkdocs(self): | ||
return Mkdocs( | ||
configuration=None, | ||
fail_on_warning=False, | ||
) | ||
|
||
|
||
class BuildConfigV2(BuildConfigBase): | ||
|
||
|
@@ -566,6 +573,8 @@ def validate(self): | |
self.validate_doc_types() | ||
self._config['mkdocs'] = self.validate_mkdocs() | ||
self._config['sphinx'] = self.validate_sphinx() | ||
# TODO: remove later | ||
self.validate_final_doc_type() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to remove this later #4638 (there is a note with the commit there) |
||
self._config['submodules'] = self.validate_submodules() | ||
|
||
def validate_formats(self): | ||
|
@@ -806,6 +815,23 @@ def validate_sphinx(self): | |
|
||
return sphinx | ||
|
||
def validate_final_doc_type(self): | ||
""" | ||
Validates that the doctype is the same as the admin panel. | ||
|
||
This a temporal validation, as the configuration file | ||
should support per version doctype, but we need to | ||
adapt the rtd code for that. | ||
""" | ||
if self.doctype != self.defaults.get('doctype', 'sphinx'): | ||
key = 'mkdocs' if self.doctype == 'mkdocs' else 'sphinx' | ||
self.error( | ||
key, | ||
'Your documentation type should match ' | ||
'the one from the admin panel of your project.', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: "project admin dashboard" instead of "admin panel of your panel" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, perhaps instead mention the expected value here? "Your project is configured as '{ project.documentation_type }' in your project admin dashboard, but there was no "sphinx" key specified in the configuration." Something like that |
||
code=INVALID_KEYS_COMBINATION, | ||
) | ||
|
||
def validate_submodules(self): | ||
""" | ||
Validates the submodules key. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1316,7 +1316,10 @@ def test_sphinx_is_default_doc_type(self): | |
('htmldir', 'sphinx_htmldir'), | ||
('singlehtml', 'sphinx_singlehtml')]) | ||
def test_sphinx_builder_check_valid(self, value, expected): | ||
build = self.get_build_config({'sphinx': {'builder': value}}) | ||
build = self.get_build_config( | ||
{'sphinx': {'builder': value}}, | ||
{'defaults': {'doctype': expected}}, | ||
) | ||
build.validate() | ||
assert build.sphinx.builder == expected | ||
assert build.doctype == expected | ||
|
@@ -1333,6 +1336,7 @@ def test_sphinx_builder_default(self): | |
build.validate() | ||
build.sphinx.builder == 'sphinx' | ||
|
||
@pytest.mark.skip | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this skipped? Perhaps we should get in the habit of code commenting why a test is xfail, skip, etc. Or i think you can do: @pytest.mark.skip("Skipped because ...") There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those tests are incompatible with the new behavior of checking the same doctype, all those changes are part of #4638 (comment) |
||
def test_sphinx_builder_ignores_default(self): | ||
build = self.get_build_config( | ||
{}, | ||
|
@@ -1454,6 +1458,7 @@ def test_mkdocs_configuration_check_valid(self, tmpdir): | |
apply_fs(tmpdir, {'mkdocs.yml': ''}) | ||
build = self.get_build_config( | ||
{'mkdocs': {'configuration': 'mkdocs.yml'}}, | ||
{'defaults': {'doctype': 'mkdocs'}}, | ||
source_file=str(tmpdir.join('readthedocs.yml')), | ||
) | ||
build.validate() | ||
|
@@ -1472,40 +1477,67 @@ def test_mkdocs_configuration_check_invalid(self, tmpdir): | |
assert excinfo.value.key == 'mkdocs.configuration' | ||
|
||
def test_mkdocs_configuration_allow_null(self): | ||
build = self.get_build_config({'mkdocs': {'configuration': None}},) | ||
build = self.get_build_config( | ||
{'mkdocs': {'configuration': None}}, | ||
{'defaults': {'doctype': 'mkdocs'}}, | ||
) | ||
build.validate() | ||
assert build.mkdocs.configuration is None | ||
|
||
def test_mkdocs_configuration_check_default(self): | ||
build = self.get_build_config({'mkdocs': {}}) | ||
build = self.get_build_config( | ||
{'mkdocs': {}}, | ||
{'defaults': {'doctype': 'mkdocs'}}, | ||
) | ||
build.validate() | ||
assert build.mkdocs.configuration is None | ||
|
||
@pytest.mark.parametrize('value', [[], True, 0, {}]) | ||
def test_mkdocs_configuration_validate_type(self, value): | ||
build = self.get_build_config({'mkdocs': {'configuration': value}},) | ||
build = self.get_build_config( | ||
{'mkdocs': {'configuration': value}}, | ||
{'defaults': {'doctype': 'mkdocs'}}, | ||
) | ||
with raises(InvalidConfig) as excinfo: | ||
build.validate() | ||
assert excinfo.value.key == 'mkdocs.configuration' | ||
|
||
@pytest.mark.parametrize('value', [True, False]) | ||
def test_mkdocs_fail_on_warning_check_valid(self, value): | ||
build = self.get_build_config({'mkdocs': {'fail_on_warning': value}}) | ||
build = self.get_build_config( | ||
{'mkdocs': {'fail_on_warning': value}}, | ||
{'defaults': {'doctype': 'mkdocs'}}, | ||
) | ||
build.validate() | ||
assert build.mkdocs.fail_on_warning is value | ||
|
||
@pytest.mark.parametrize('value', [[], 'invalid', 5]) | ||
def test_mkdocs_fail_on_warning_check_invalid(self, value): | ||
build = self.get_build_config({'mkdocs': {'fail_on_warning': value}}) | ||
build = self.get_build_config( | ||
{'mkdocs': {'fail_on_warning': value}}, | ||
{'defaults': {'doctype': 'mkdocs'}}, | ||
) | ||
with raises(InvalidConfig) as excinfo: | ||
build.validate() | ||
assert excinfo.value.key == 'mkdocs.fail_on_warning' | ||
|
||
def test_mkdocs_fail_on_warning_check_default(self): | ||
build = self.get_build_config({'mkdocs': {}}) | ||
build = self.get_build_config( | ||
{'mkdocs': {}}, | ||
{'defaults': {'doctype': 'mkdocs'}}, | ||
) | ||
build.validate() | ||
assert build.mkdocs.fail_on_warning is False | ||
|
||
def test_validates_different_filetype(self): | ||
build = self.get_build_config( | ||
{'mkdocs': {}}, | ||
{'defaults': {'doctype': 'sphinx'}}, | ||
) | ||
with raises(InvalidConfig) as excinfo: | ||
build.validate() | ||
assert excinfo.value.key == 'mkdocs' | ||
|
||
@pytest.mark.parametrize('value', [[], 'invalid', 0]) | ||
def test_submodules_check_invalid_type(self, value): | ||
build = self.get_build_config({'submodules': value}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,14 +52,15 @@ def __init__(self, *args, **kwargs): | |
|
||
def get_yaml_config(self): | ||
"""Find the ``mkdocs.yml`` file in the project root.""" | ||
# TODO: try to load from the configuration file first. | ||
test_path = os.path.join( | ||
self.project.checkout_path(self.version.slug), | ||
'mkdocs.yml' | ||
) | ||
if os.path.exists(test_path): | ||
return test_path | ||
return None | ||
mkdoc_path = self.config.mkdocs.configuration | ||
if not mkdoc_path: | ||
mkdoc_path = os.path.join( | ||
self.project.checkout_path(self.version.slug), | ||
'mkdocs.yml' | ||
) | ||
if not os.path.exists(mkdoc_path): | ||
return None | ||
return mkdoc_path | ||
|
||
def load_yaml_config(self): | ||
""" | ||
|
@@ -184,6 +185,8 @@ def build(self): | |
'--site-dir', self.build_dir, | ||
'--config-file', self.yaml_file, | ||
] | ||
if self.config.mkdocs.fail_on_warning: | ||
build_command.append('--strict') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I'd let users configure this in their There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm +1 for this, I didn't know about that, Sphinx doesn't have something like that (I mean, using the conf.py file). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could define |
||
cmd_ret = self.run( | ||
*build_command, | ||
cwd=checkout_path, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -236,7 +236,7 @@ def install_core_requirements(self): | |
'recommonmark==0.4.0', | ||
] | ||
|
||
if self.project.documentation_type == 'mkdocs': | ||
if self.config.doctype == 'mkdocs': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm leaving the doctype access in some places where we always use the config object, it doesn't break anything anyway. Just keeping the same pattern. |
||
requirements.append('mkdocs==0.17.3') | ||
else: | ||
# We will assume semver here and only automate up to the next | ||
|
@@ -275,7 +275,7 @@ def install_core_requirements(self): | |
def install_user_requirements(self): | ||
requirements_file_path = self.config.python.requirements | ||
if not requirements_file_path and requirements_file_path != '': | ||
builder_class = get_builder_class(self.project.documentation_type) | ||
builder_class = get_builder_class(self.config.doctype) | ||
docs_dir = (builder_class(build_env=self.build_env, python_env=self) | ||
.docs_dir()) | ||
paths = [docs_dir, ''] | ||
|
@@ -352,7 +352,7 @@ def install_core_requirements(self): | |
'recommonmark', | ||
] | ||
|
||
if self.project.documentation_type == 'mkdocs': | ||
if self.config.doctype == 'mkdocs': | ||
pip_requirements.append('mkdocs') | ||
else: | ||
pip_requirements.append('readthedocs-sphinx-ext') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -318,6 +318,11 @@ def save(self, *args, **kwargs): # pylint: disable=arguments-differ | |
self.slug = slugify(self.name) | ||
if self.slug == '': | ||
raise Exception(_('Model must have slug')) | ||
if self.documentation_type == 'auto': | ||
# This used to determine the type and automatically set the | ||
# documentation type to Sphinx for rST and Mkdocs for markdown. | ||
# It now just forces Sphinx, due to markdown support. | ||
self.documentation_type = 'sphinx' | ||
super(Project, self).save(*args, **kwargs) | ||
for owner in self.users.all(): | ||
assign('view_project', owner, self) | ||
|
@@ -590,16 +595,6 @@ def conf_dir(self, version=LATEST): | |
if conf_file: | ||
return os.path.dirname(conf_file) | ||
|
||
@property | ||
def is_type_sphinx(self): | ||
"""Is project type Sphinx.""" | ||
return 'sphinx' in self.documentation_type | ||
|
||
@property | ||
def is_type_mkdocs(self): | ||
"""Is project type Mkdocs.""" | ||
return 'mkdocs' in self.documentation_type | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing these is a first step to deprecate Project.doctype |
||
@property | ||
def is_imported(self): | ||
return bool(self.repo) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -470,10 +470,6 @@ def run_build(self, docker, record): | |
|
||
# Environment used for building code, usually with Docker | ||
with self.build_env: | ||
|
||
if self.project.documentation_type == 'auto': | ||
self.update_documentation_type() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I move this logic to the save method of the project model |
||
|
||
python_env_cls = Virtualenv | ||
if self.config.conda is not None: | ||
self._log('Using conda') | ||
|
@@ -589,21 +585,6 @@ def set_valid_clone(self): | |
self.project.has_valid_clone = True | ||
self.version.project.has_valid_clone = True | ||
|
||
def update_documentation_type(self): | ||
""" | ||
Force Sphinx for 'auto' documentation type. | ||
|
||
This used to determine the type and automatically set the documentation | ||
type to Sphinx for rST and Mkdocs for markdown. It now just forces | ||
Sphinx, due to markdown support. | ||
""" | ||
ret = 'sphinx' | ||
project_data = api_v2.project(self.project.pk).get() | ||
project_data['documentation_type'] = ret | ||
api_v2.project(self.project.pk).put(project_data) | ||
self.project.documentation_type = ret | ||
self.version.project.documentation_type = ret | ||
|
||
def update_app_instances(self, html=False, localmedia=False, search=False, | ||
pdf=False, epub=False): | ||
""" | ||
|
@@ -642,7 +623,10 @@ def update_app_instances(self, html=False, localmedia=False, search=False, | |
pdf=pdf, | ||
epub=epub, | ||
), | ||
callback=sync_callback.s(version_pk=self.version.pk, commit=self.build['commit']), | ||
callback=sync_callback.s( | ||
version_pk=self.version.pk, | ||
commit=self.build['commit'], | ||
), | ||
) | ||
|
||
def setup_python_environment(self): | ||
|
@@ -714,10 +698,12 @@ def build_docs_html(self): | |
|
||
# Gracefully attempt to move files via task on web workers. | ||
try: | ||
broadcast(type='app', task=move_files, | ||
args=[self.version.pk, socket.gethostname()], | ||
kwargs=dict(html=True) | ||
) | ||
broadcast( | ||
type='app', | ||
task=move_files, | ||
args=[self.version.pk, socket.gethostname()], | ||
kwargs=dict(html=True) | ||
) | ||
except socket.error: | ||
log.exception('move_files task has failed on socket error.') | ||
|
||
|
@@ -729,23 +715,23 @@ def build_docs_localmedia(self): | |
return False | ||
|
||
if self.build_localmedia: | ||
if self.project.is_type_sphinx: | ||
if self.is_type_sphinx(): | ||
return self.build_docs_class('sphinx_singlehtmllocalmedia') | ||
return False | ||
|
||
def build_docs_pdf(self): | ||
"""Build PDF docs.""" | ||
if ('pdf' not in self.config.formats or | ||
self.project.slug in HTML_ONLY or | ||
not self.project.is_type_sphinx): | ||
self.project.slug in HTML_ONLY or | ||
not self.is_type_sphinx()): | ||
return False | ||
return self.build_docs_class('sphinx_pdf') | ||
|
||
def build_docs_epub(self): | ||
"""Build ePub docs.""" | ||
if ('epub' not in self.config.formats or | ||
self.project.slug in HTML_ONLY or | ||
not self.project.is_type_sphinx): | ||
not self.is_type_sphinx()): | ||
return False | ||
return self.build_docs_class('sphinx_epub') | ||
|
||
|
@@ -769,6 +755,10 @@ def send_notifications(self): | |
"""Send notifications on build failure.""" | ||
send_notifications.delay(self.version.pk, build_pk=self.build['id']) | ||
|
||
def is_type_sphinx(self): | ||
"""Is documentation type Sphinx.""" | ||
return 'sphinx' in self.config.doctype | ||
|
||
|
||
# Web tasks | ||
@app.task(queue='web') | ||
|
@@ -816,8 +806,8 @@ def sync_files(project_pk, version_pk, hostname=None, html=False, | |
|
||
|
||
@app.task(queue='web') | ||
def move_files(version_pk, hostname, html=False, localmedia=False, search=False, | ||
pdf=False, epub=False): | ||
def move_files(version_pk, hostname, html=False, localmedia=False, | ||
search=False, pdf=False, epub=False): | ||
""" | ||
Task to move built documentation to web servers. | ||
|
||
|
@@ -912,11 +902,13 @@ def update_search(version_pk, commit, delete_non_commit_files=True): | |
""" | ||
version = Version.objects.get(pk=version_pk) | ||
|
||
if version.project.is_type_sphinx: | ||
if 'sphinx' in version.project.documentation_type: | ||
page_list = process_all_json_files(version, build_dir=False) | ||
else: | ||
log.debug('Unknown documentation type: %s', | ||
version.project.documentation_type) | ||
log.debug( | ||
'Unknown documentation type: %s', | ||
version.project.documentation_type | ||
) | ||
return | ||
|
||
log_msg = ' '.join([page['path'] for page in page_list]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were showing the full path here (/rtd/builds/...)