-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Migrate /translations
route to core
#83280
Migrate /translations
route to core
#83280
Conversation
// setup i18n prior to any other service, to have translations ready | ||
const i18nServiceSetup = await this.i18n.setup({ http: httpSetup, pluginPaths }); |
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.
Had to move this after the http
service setup to register the route. I could move i18n initialization in another method (like we did for plugin.discover
) to keep the i18n setup before context and http, but I don't think this is really necessary
export default function ({ getService }: FtrProviderContext) { | ||
const supertest = getService('supertest'); | ||
|
||
describe('compression', () => { | ||
it(`uses compression when there isn't a referer`, async () => { |
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.
Extracted / converted to TS from test/api_integration/apis/core/index.js
headers: { | ||
'content-type': 'application/json', | ||
'cache-control': 'must-revalidate', | ||
etag: translationCache.hash, | ||
}, | ||
body: translationCache.translations, |
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.
As discussed with @restrry, we ideally should use cache-control
instead of etag
here in dist mode to avoid an unnecessary roundtrip. However as this requires some refactoring to be able to add a path prefix,suffix or query param to invalidate the request when the kibana version is bumped (as we do with static assets), it will be done at a later time. I will create an issue for that before the current PR got merged.
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.
Created #83409
💚 Build SucceededMetrics [docs]Distributable file count
History
To update your PR or re-run it, just comment with: |
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.
tested locally, looks good
* move i18n route to core * add FTR test for endpoint
* master: Migrate `/translations` route to core (elastic#83280) [APM] Ensure APM jest script can run (elastic#83398) [Uptime] Monitor status alert use url as instance (elastic#81736) [ML] Add basic license test run details to ML+Transform READMEs (elastic#83259) TSVB doesn't communicate it's index-patterns to dashboard (elastic#82964) [Alerting UI] Added ability to assign alert actions to resolved action group in UI (elastic#83139) Skips Vega test skip flaky suite (elastic#79967) [bundle optimization] Update to semver 7.x to get tree-shaking (elastic#83020) Added ability to fire actions when an alert instance is resolved (elastic#82799) [ML] Adds functional tests for the index data visualizer card contents (elastic#83174)
* move i18n route to core * add FTR test for endpoint
… into add-logs-to-node-details * 'add-logs-to-node-details' of github.com:phillipb/kibana: fix tall vislib charts in visualize (elastic#83340) [Lens] Avoid unnecessary data fetching on dimension flyout open (elastic#82957) [Security Solution][Case] Change case connector minimum required license to basic (elastic#83401) fix logstash central pipeline management test (elastic#83281) [Search] Send to background UI (elastic#81793) Migrate `/translations` route to core (elastic#83280) [APM] Ensure APM jest script can run (elastic#83398) [Uptime] Monitor status alert use url as instance (elastic#81736) [ML] Add basic license test run details to ML+Transform READMEs (elastic#83259) TSVB doesn't communicate it's index-patterns to dashboard (elastic#82964) [Alerting UI] Added ability to assign alert actions to resolved action group in UI (elastic#83139) Skips Vega test skip flaky suite (elastic#79967) [bundle optimization] Update to semver 7.x to get tree-shaking (elastic#83020) Added ability to fire actions when an alert instance is resolved (elastic#82799)
Summary
Follow-up of #81799
Migrate the
/translations/{locale}.json
route to core.Checklist