-
Notifications
You must be signed in to change notification settings - Fork 35
Generate json artifacts #43
Changes from 4 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,51 @@ | |
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 import logging | ||
from sphinx.util.console import bold | ||
|
||
from .embed import EmbedDirective | ||
from .mixins import BuilderMixin | ||
|
||
log = logging.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 | ||
KEYS = [ | ||
'body', | ||
'prev', | ||
'display_toc', | ||
'title', | ||
'sourcename', | ||
'customsidebar', | ||
'current_page_name', | ||
'next', | ||
'sidebars', | ||
'metatags', | ||
'meta', | ||
'parents', | ||
'toc', | ||
'alabaster_version', | ||
'page_source_suffix' | ||
] | ||
|
||
|
||
def finalize_media(app): | ||
""" Point media files at our media server. """ | ||
|
||
|
@@ -126,6 +151,31 @@ def rtd_render(self, template, render_context): | |
app.builder.templates) | ||
|
||
|
||
def generate_json_artifacts(app, pagename, templatename, context, doctree): | ||
try: | ||
# We need to get the output directory where the docs are built | ||
# _build/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. Ok, I review a little more the code, this is copied to the artifacts folder in a later step, so we need to copy those there, or maybe do this in the |
||
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 + '.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. The extension for the previous build is |
||
outdir = os.path.dirname(outjson) | ||
if not os.path.exists(outdir): | ||
os.makedirs(outdir) | ||
with open(outjson, 'w+') as jfile: | ||
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 | ||
} | ||
jfile.write(json.dumps(to_context, indent=4)) | ||
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 +263,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) | ||
|
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.
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 comment
The 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.