Skip to content

Force to use proxied API for footer and search #6768

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

Merged
merged 4 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 1 addition & 9 deletions readthedocs/core/static-src/core/js/doc-embed/footer.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,9 @@ function init() {
get_data['subproject'] = true;
}

// Check for new logic around proxying requests.
// ``proxied_api_host`` won't exist on existing built docs,
// so default to ``api_host`` in those cases
var real_api_host = rtd.api_host;
if ("proxied_api_host" in rtd) {
real_api_host = rtd.proxied_api_host;
}

// Get footer HTML from API and inject it into the page.
$.ajax({
url: real_api_host + "/api/v2/footer_html/",
url: rtd.proxied_api_host + "/api/v2/footer_html/",
crossDomain: true,
xhrFields: {
withCredentials: true,
Expand Down
3 changes: 3 additions & 0 deletions readthedocs/core/static-src/core/js/doc-embed/rtd-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ function get() {

$.extend(config, defaults, window.READTHEDOCS_DATA);

// Force to use new settings
config.proxied_api_host = '/_';

return config;
}

Expand Down
4 changes: 1 addition & 3 deletions readthedocs/core/static-src/core/js/doc-embed/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,12 @@ function attach_elastic_search_query(data) {
var project = data.project;
var version = data.version;
var language = data.language || 'en';
var api_host = data.api_host;

var query_override = function (query) {
var search_def = $.Deferred();
var search_url = document.createElement('a');

search_url.href = api_host;
search_url.pathname = '/api/v2/docsearch/';
search_url.href = data.proxied_api_host + '/api/v2/docsearch/';
search_url.search = '?q=' + $.urlencode(query) + '&project=' + project +
'&version=' + version + '&language=' + language;

Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/static/core/js/readthedocs-doc-embed.js

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion readthedocs/doc_builder/backends/mkdocs.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ def generate_rtd_data(self, docs_dir, mkdocs_config):
'docroot': docs_dir,
'source_suffix': '.md',
'api_host': settings.PUBLIC_API_URL,
'proxied_api_host': settings.RTD_PROXIED_API_URL,
'ad_free': not self.project.show_advertising,
'commit': self.version.project.vcs_repo(self.version.slug).commit,
'global_analytics_code': settings.GLOBAL_ANALYTICS_CODE,
Expand Down
1 change: 0 additions & 1 deletion readthedocs/doc_builder/backends/sphinx.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ def get_config_params(self):
'settings': settings,
'conf_py_path': conf_py_path,
'api_host': settings.PUBLIC_API_URL,
'proxied_api_host': settings.RTD_PROXIED_API_URL,
'commit': self.project.vcs_repo(self.version.slug).commit,
'versions': versions,
'downloads': downloads,
Expand Down
1 change: 0 additions & 1 deletion readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ context = {
'single_version': {{ project.single_version }},
'conf_py_path': '{{ conf_py_path }}',
'api_host': '{{ api_host }}',
'proxied_api_host': '{{ proxied_api_host }}',
'github_user': '{{ github_user }}',
'github_repo': '{{ github_repo }}',
'github_version': '{{ github_version }}',
Expand Down
4 changes: 0 additions & 4 deletions readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ class CommunityBaseSettings(Settings):
PUBLIC_DOMAIN_USES_HTTPS = False
USE_SUBDOMAIN = False
PUBLIC_API_URL = 'https://{}'.format(PRODUCTION_DOMAIN)
# Some endpoints from the API can be proxied on other domain
# or use the same domain where the docs are being served
# (omit the host if that's the case).
RTD_PROXIED_API_URL = PUBLIC_API_URL
RTD_EXTERNAL_VERSION_DOMAIN = 'external-builds.readthedocs.io'

# Doc Builder Backends
Expand Down
1 change: 0 additions & 1 deletion readthedocs/settings/docker_compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class DockerBaseSettings(CommunityDevSettings):
PRODUCTION_DOMAIN = 'community.dev.readthedocs.io'
PUBLIC_DOMAIN = 'community.dev.readthedocs.io'
PUBLIC_API_URL = f'http://{PRODUCTION_DOMAIN}'
RTD_PROXIED_API_URL = PUBLIC_API_URL
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we keep the setting, to make it easier for users to override? I think supporting the setting is fine, I just want a default that is _/ as we have now.

Copy link
Member Author

Choose a reason for hiding this comment

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

We had that setting to pass it down to context during build time. But we want to apply this change to everyone. So, not sure if we can have both.

SLUMBER_API_HOST = 'http://web:8000'
RTD_EXTERNAL_VERSION_DOMAIN = 'org.dev.readthedocs.build'

Expand Down