Skip to content

Remove localmendia and search parameters #4895

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

Merged
merged 2 commits into from
Dec 26, 2018
Merged

Remove localmendia and search parameters #4895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
621b474
Remove localmendia and search parameters
stsewd Nov 12, 2018
d8a8967
Tests for mkdocs
stsewd Nov 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 21 additions & 28 deletions readthedocs/projects/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@

log = logging.getLogger(__name__)

HTML_ONLY = getattr(settings, 'HTML_ONLY_PROJECTS', ())
Copy link
Member Author

Choose a reason for hiding this comment

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

I can see that we have 3 projects here, with a comment banned, I think we can move those to use the skip field on the db.

Copy link
Contributor

Choose a reason for hiding this comment

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

skip will skip the build entirely, sounds like we want to disable non-html builds instead. i don't think we have a way to opt users into disabling pdf/epub from our side other than this setting right now. it could just be a feature flag or something though.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you check the settings from the ops repo, there is this comment banned, and if these projects are abusive projects, probably the best to do is ban them (skip)? otherwise, the builders would time out or kill the process when building pdf/epub anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like of the 3 projects, 1 is deleted, and the others haven't been built for 4 months and 1 year, so I think we're probably safe to remove it, and deal with it if it hit issues again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems fine to me



class SyncRepositoryMixin(object):

Expand Down Expand Up @@ -266,13 +264,11 @@ class UpdateDocsTaskStep(SyncRepositoryMixin):
"""

def __init__(self, build_env=None, python_env=None, config=None,
force=False, search=True, localmedia=True,
build=None, project=None, version=None, task=None):
force=False, build=None, project=None,
version=None, task=None):
self.build_env = build_env
self.python_env = python_env
self.build_force = force
self.build_search = search
self.build_localmedia = localmedia
self.build = {}
if build is not None:
self.build = build
Expand All @@ -289,7 +285,7 @@ def __init__(self, build_env=None, python_env=None, config=None,

# pylint: disable=arguments-differ
def run(self, pk, version_pk=None, build_pk=None, record=True,
docker=None, search=True, force=False, localmedia=True, **__):
docker=None, force=False, **__):
"""
Run a documentation sync n' build.

Expand All @@ -312,9 +308,7 @@ def run(self, pk, version_pk=None, build_pk=None, record=True,
:param record bool: record a build object in the database
:param docker bool: use docker to build the project (if ``None``,
``settings.DOCKER_ENABLE`` is used)
:param search bool: update search
:param force bool: force Sphinx build
:param localmedia bool: update localmedia

:returns: whether build was successful or not

Expand All @@ -327,8 +321,6 @@ def run(self, pk, version_pk=None, build_pk=None, record=True,
self.project = self.get_project(pk)
self.version = self.get_version(self.project, version_pk)
self.build = self.get_build(build_pk)
self.build_search = search
self.build_localmedia = localmedia
self.build_force = force
self.config = None

Expand Down Expand Up @@ -769,37 +761,38 @@ def build_docs_html(self):

def build_docs_search(self):
"""Build search data."""
# TODO rely on config parameter here when Project.documentation_type is
# removed in #4638. Mkdocs has no search currently
if self.project.documentation_type == 'mkdocs':
return False
return self.build_search
# Search is always run in sphinx using the rtd-sphinx-extension.
# Mkdocs has no search currently.
if self.is_type_sphinx():
return True
return False

def build_docs_localmedia(self):
"""Get local media files with separate build."""
if 'htmlzip' not in self.config.formats:
return False

if self.build_localmedia:
if self.is_type_sphinx():
return self.build_docs_class('sphinx_singlehtmllocalmedia')
# We don't generate a zip for mkdocs currently.
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.is_type_sphinx()):
if 'pdf' not in self.config.formats:
return False
return self.build_docs_class('sphinx_pdf')
# Mkdocs has no pdf generation currently.
if self.is_type_sphinx():
return self.build_docs_class('sphinx_pdf')
return False

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.is_type_sphinx()):
if 'epub' not in self.config.formats:
return False
return self.build_docs_class('sphinx_epub')
# Mkdocs has no epub generation currently.
if self.is_type_sphinx():
return self.build_docs_class('sphinx_epub')
return False

def build_docs_class(self, builder_class):
"""
Expand Down
10 changes: 10 additions & 0 deletions readthedocs/rtd_tests/mocks/environment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def __init__(self):
'readthedocs.doc_builder.backends.sphinx.HtmlBuilder.build'),
'html_move': mock.patch(
'readthedocs.doc_builder.backends.sphinx.HtmlBuilder.move'),
'localmedia_build': mock.patch(
'readthedocs.doc_builder.backends.sphinx.LocalMediaBuilder.build'),
'localmedia_move': mock.patch(
'readthedocs.doc_builder.backends.sphinx.LocalMediaBuilder.move'),
'pdf_build': mock.patch(
'readthedocs.doc_builder.backends.sphinx.PdfBuilder.build'),
'pdf_move': mock.patch(
Expand All @@ -42,6 +46,12 @@ def __init__(self):
'readthedocs.doc_builder.backends.sphinx.EpubBuilder.build'),
'epub_move': mock.patch(
'readthedocs.doc_builder.backends.sphinx.EpubBuilder.move'),
'move_mkdocs': mock.patch(
'readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.move'),
'append_conf_mkdocs': mock.patch(
'readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.append_conf'),
'html_build_mkdocs': mock.patch(
'readthedocs.doc_builder.backends.mkdocs.MkdocsHTML.build'),
'glob': mock.patch('readthedocs.doc_builder.backends.sphinx.glob'),

'docker': mock.patch('readthedocs.doc_builder.environments.APIClient'),
Expand Down
70 changes: 58 additions & 12 deletions readthedocs/rtd_tests/tests/test_builds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ def test_build(self, load_config):
build_env = LocalBuildEnvironment(project=project, version=version, build={})
python_env = Virtualenv(version=version, build_env=build_env)
config = load_yaml_config(version)
task = UpdateDocsTaskStep(build_env=build_env, project=project, python_env=python_env,
version=version, search=False, localmedia=False, config=config)
task = UpdateDocsTaskStep(
build_env=build_env, project=project, python_env=python_env,
version=version, config=config
)
task.build_docs()

# Get command and check first part of command list is a call to sphinx
Expand All @@ -74,8 +76,10 @@ def test_build_respects_pdf_flag(self, load_config):
build_env = LocalBuildEnvironment(project=project, version=version, build={})
python_env = Virtualenv(version=version, build_env=build_env)
config = load_yaml_config(version)
task = UpdateDocsTaskStep(build_env=build_env, project=project, python_env=python_env,
version=version, search=False, localmedia=False, config=config)
task = UpdateDocsTaskStep(
build_env=build_env, project=project, python_env=python_env,
version=version, config=config
)

task.build_docs()

Expand All @@ -85,6 +89,40 @@ def test_build_respects_pdf_flag(self, load_config):
# PDF however was disabled and therefore not built.
self.assertFalse(self.mocks.epub_build.called)

@mock.patch('readthedocs.doc_builder.config.load_config')
def test_dont_localmedia_build_pdf_epub_search_in_mkdocs(self, load_config):
load_config.side_effect = create_load()
project = get(
Project,
slug='project-1',
documentation_type='mkdocs',
enable_pdf_build=True,
enable_epub_build=True,
versions=[fixture()]
)
version = project.versions.all().first()

build_env = LocalBuildEnvironment(
project=project,
version=version,
build={}
)
python_env = Virtualenv(version=version, build_env=build_env)
config = load_yaml_config(version)
task = UpdateDocsTaskStep(
build_env=build_env, project=project, python_env=python_env,
version=version, config=config
)

task.build_docs()

# Only html for mkdocs was built
self.mocks.html_build_mkdocs.assert_called_once()
self.mocks.html_build.assert_not_called()
self.mocks.localmedia_build.assert_not_called()
self.mocks.pdf_build.assert_not_called()
self.mocks.epub_build.assert_not_called()

@mock.patch('readthedocs.doc_builder.config.load_config')
def test_build_respects_epub_flag(self, load_config):
'''Test build with epub enabled'''
Expand All @@ -101,8 +139,10 @@ def test_build_respects_epub_flag(self, load_config):
build_env = LocalBuildEnvironment(project=project, version=version, build={})
python_env = Virtualenv(version=version, build_env=build_env)
config = load_yaml_config(version)
task = UpdateDocsTaskStep(build_env=build_env, project=project, python_env=python_env,
version=version, search=False, localmedia=False, config=config)
task = UpdateDocsTaskStep(
build_env=build_env, project=project, python_env=python_env,
version=version, config=config
)
task.build_docs()

# The HTML and the Epub format were built.
Expand All @@ -128,8 +168,10 @@ def test_build_respects_yaml(self, load_config):
python_env = Virtualenv(version=version, build_env=build_env)

config = load_yaml_config(version)
task = UpdateDocsTaskStep(build_env=build_env, project=project, python_env=python_env,
version=version, search=False, localmedia=False, config=config)
task = UpdateDocsTaskStep(
build_env=build_env, project=project, python_env=python_env,
version=version, config=config
)
task.build_docs()

# The HTML and the Epub format were built.
Expand Down Expand Up @@ -159,8 +201,10 @@ def test_build_pdf_latex_failures(self, load_config):
build_env = LocalBuildEnvironment(project=project, version=version, build={})
python_env = Virtualenv(version=version, build_env=build_env)
config = load_yaml_config(version)
task = UpdateDocsTaskStep(build_env=build_env, project=project, python_env=python_env,
version=version, search=False, localmedia=False, config=config)
task = UpdateDocsTaskStep(
build_env=build_env, project=project, python_env=python_env,
version=version, config=config
)

# Mock out the separate calls to Popen using an iterable side_effect
returns = [
Expand Down Expand Up @@ -203,8 +247,10 @@ def test_build_pdf_latex_not_failure(self, load_config):
build_env = LocalBuildEnvironment(project=project, version=version, build={})
python_env = Virtualenv(version=version, build_env=build_env)
config = load_yaml_config(version)
task = UpdateDocsTaskStep(build_env=build_env, project=project, python_env=python_env,
version=version, search=False, localmedia=False, config=config)
task = UpdateDocsTaskStep(
build_env=build_env, project=project, python_env=python_env,
version=version, config=config
)

# Mock out the separate calls to Popen using an iterable side_effect
returns = [
Expand Down