Skip to content
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

Only call language service when language not defaulted #1098

Closed
wants to merge 1 commit into from

Conversation

orangejulius
Copy link
Member

@orangejulius orangejulius commented Mar 13, 2018

The default language is English, which is also what is stored in Elasticsearch. Thus, if the user did not specify a language via headers or query parameter, there is no reason to call Placeholder.

Some Placeholder responses for language translation can be 30KB, and all that JSON takes considerable time to parse

This was originally part of #1095

Edit: the default language is not English in WOF/Placeholder.

The default language is English, which is also what is stored in
Elasticsearch. Thus, if the user did not specify a language via headers
or query parameter, there is no reason to call Placeholder.

Some Placeholder responses for language translation can be 30KB, and all
that JSON takes considerable time to parse
@orangejulius
Copy link
Member Author

orangejulius commented May 1, 2018

Upon further reflection, we may not want to merge this PR. Instead, we can disable storing, but not indexing, of the admin name fields in Elasticsearch. This should drastically cut down on the amount of data stored in Elasticsearch (all records in the USA store a duplicate copy of the string "United States", for example).

For simple get requests Placeholder is quite fast, and if needed we can cache commonly used WOF records in the API (although that brings complications), so the tradeoff of an extra request to a fast service to possibly improve the efficiency of Elasticsearch by quite a bit should be worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants