-
Notifications
You must be signed in to change notification settings - Fork 35
Generate json artifacts #43
Changes from 12 commits
a0091db
9fb97cf
5413c6e
be06d24
c853064
dc33983
ffc0ffe
c5bbd70
ddfba51
a59aad8
6edf54a
c659280
ebc2832
4fc6363
ef2564f
cc7c7f8
fb33462
38f9999
b0643cb
dff342f
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 |
---|---|---|
|
@@ -3,26 +3,58 @@ | |
from __future__ import absolute_import | ||
|
||
import codecs | ||
import json | ||
import os | ||
import re | ||
import types | ||
from distutils.version import LooseVersion | ||
|
||
import sphinx | ||
from sphinx import package_dir | ||
from sphinx.builders.html import StandaloneHTMLBuilder, DirectoryHTMLBuilder, SingleFileHTMLBuilder | ||
from sphinx.builders.html import (DirectoryHTMLBuilder, SingleFileHTMLBuilder, | ||
StandaloneHTMLBuilder) | ||
from sphinx.util.console import bold | ||
|
||
from .embed import EmbedDirective | ||
from .mixins import BuilderMixin | ||
|
||
try: | ||
# Avaliable from Sphinx 1.6 | ||
from sphinx.util.logging import getLogger | ||
except ImportError: | ||
from logging import getLogger | ||
|
||
log = getLogger(__name__) | ||
|
||
MEDIA_MAPPING = { | ||
"_static/jquery.js": "%sjavascript/jquery/jquery-2.0.3.min.js", | ||
"_static/underscore.js": "%sjavascript/underscore.js", | ||
"_static/doctools.js": "%sjavascript/doctools.js", | ||
} | ||
|
||
|
||
# Whitelist keys that we want to output | ||
# to the json artifacts. | ||
KEYS = [ | ||
'body', | ||
'alabaster_version', | ||
'display_toc', | ||
'title', | ||
'sourcename', | ||
'customsidebar', | ||
'metatags', | ||
'current_page_name', | ||
'next', | ||
'rellinks', | ||
'meta', | ||
'parents', | ||
'sidebars', | ||
'toc', | ||
'prev', | ||
'page_source_suffix', | ||
] | ||
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. Looking into the code https://github.com/rtfd/readthedocs.org/blob/04ce7bb7a65a5d9ce2ad5a7b9a34185ef45c98ce/readthedocs/search/parse_json.py#L92-L124, we don't need all these keys. We only need:
Should I remove the rest? Is it used in other parts of the code that I'm missing? 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. They are the only ones that we use currently. If there is other useful data, we should keep it, but I believe that's the primary data that we care about. |
||
|
||
|
||
def finalize_media(app): | ||
""" Point media files at our media server. """ | ||
|
||
|
@@ -126,6 +158,36 @@ def rtd_render(self, template, render_context): | |
app.builder.templates) | ||
|
||
|
||
def generate_json_artifacts(app, pagename, templatename, context, doctree): | ||
""" | ||
Generate JSON artifacts for each page. | ||
|
||
This way we can skip generating this in other build step. | ||
""" | ||
try: | ||
# We need to get the output directory where the docs are built | ||
# _build/html/_json. | ||
build_json = os.path.abspath( | ||
os.path.join(app.outdir, '_json') | ||
) | ||
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 wonder if we should output this to |
||
outjson = os.path.join(build_json, pagename + '.fjson') | ||
outdir = os.path.dirname(outjson) | ||
if not os.path.exists(outdir): | ||
os.makedirs(outdir) | ||
with open(outjson, 'w+') as json_file: | ||
to_context = { | ||
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 the field should be blank rather than non existence if the key is missing. to_context = { key: context.get(key, '') for key in KEYS} 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. This makes sense, so that the JSON always has the same keys. |
||
key: context[key] | ||
for key in KEYS | ||
if key in context | ||
} | ||
json_file.write(json.dumps(to_context, indent=4)) | ||
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. what about using 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. Yeah, that's shorter |
||
log.info('{page} processed.'.format(page=outjson)) | ||
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. Maybe we want to change/remove this logging? |
||
except Exception: | ||
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 should probably use a couple different
Should probably catch them each specifically if possible, though having a wider 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. Got it |
||
log.exception( | ||
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. This doesn't make the build to fail (we need to throw the exception), it should fail? 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 we can keep it from failing for now, and just log it. |
||
'Failure in JSON search dump for {page}'.format(page=outjson) | ||
) | ||
|
||
|
||
class HtmlBuilderMixin(BuilderMixin): | ||
|
||
static_readthedocs_files = [ | ||
|
@@ -213,6 +275,7 @@ def setup(app): | |
app.add_builder(ReadtheDocsSingleFileHTMLBuilderLocalMedia) | ||
app.connect('builder-inited', finalize_media) | ||
app.connect('html-page-context', update_body) | ||
app.connect('html-page-context', generate_json_artifacts) | ||
|
||
# Embed | ||
app.add_directive('readthedocs-embed', EmbedDirective) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,12 @@ class LanguageIntegrationTests(unittest.TestCase): | |
|
||
def _run_test(self, test_dir, test_file, test_string, builder='html'): | ||
with build_output(test_dir, test_file, builder) as data: | ||
self.assertIn(test_string, data) | ||
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 replate this to test more easily the output :) |
||
if not isinstance(test_string, list): | ||
test_strings = [test_string] | ||
else: | ||
test_strings = test_string | ||
for string in test_strings: | ||
self.assertIn(string, data) | ||
|
||
|
||
class IntegrationTests(LanguageIntegrationTests): | ||
|
@@ -56,3 +61,10 @@ def test_searchtools_is_patched(self): | |
HtmlBuilderMixin.REPLACEMENT_PATTERN | ||
) | ||
self.assertIn(HtmlBuilderMixin.REPLACEMENT_TEXT, data) | ||
|
||
def test_generate_json_artifacts(self): | ||
self._run_test( | ||
'pyexample', | ||
'_build/html/_json/index.fjson', | ||
['current_page_name', 'title', 'body', 'toc'], | ||
) |
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.
Sphinx 1.4 has the logging module, but not
getLogger