Skip to content

Migrate kql telemetry and scripts api#52651

Merged
flash1293 merged 9 commits intoelastic:masterfrom
flash1293:data-apis
Dec 20, 2019
Merged

Migrate kql telemetry and scripts api#52651
flash1293 merged 9 commits intoelastic:masterfrom
flash1293:data-apis

Conversation

@flash1293
Copy link
Copy Markdown
Contributor

This PR migrates the apis for kql telemetry (opting in/out) and the scripts api returning currently active scripting languages for scripted fields to the new platform.

As they are related to the services the data plugin already owns, these services are put into the data plugin as well

@flash1293 flash1293 added Feature:NP Migration v7.6.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Dec 10, 2019
@flash1293 flash1293 marked this pull request as ready for review December 11, 2019 16:17
@flash1293 flash1293 requested a review from a team as a code owner December 11, 2019 16:17
@flash1293 flash1293 added Team:AppArch Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Dec 11, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@flash1293 flash1293 requested a review from lukeelmers December 11, 2019 16:18
@flash1293
Copy link
Copy Markdown
Contributor Author

Jenkins, test this

Copy link
Copy Markdown
Contributor

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Code LGTM; thank you for doing this! (Tested Chrome OSX and verified routes are still working)

import moment from 'moment-timezone';
import numeralLanguages from '@elastic/numeral/languages';
import { i18n } from '@kbn/i18n';
import { DEFAULT_QUERY_LANGUAGE } from '../../../plugins/data/common';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should import from public here if possible, instead of common

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This file is used on the server, so importing from public won't work. Funny enough as it's not in a server dir, importing from server also doesn't work with the current linting rules. I would like to keep common for now and we can clean it up when migrating this file

}

public setup(core: CoreSetup) {
public async setup(core: CoreSetup, { usageCollection }: DataPluginSetupDependencies) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is async necessary here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not, I used to block on telemetry service setup during development, but it doesn't make sense in practice. I made the async-ness an implementation detail.

@flash1293 flash1293 requested a review from a team as a code owner December 20, 2019 15:07
<b>Signature:</b>

```typescript
get: (request: LegacyRequest | KibanaRequest<unknown, unknown, unknown, any>) => string;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@joshdover These are the strange core API changes this PR introduces.

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@flash1293 flash1293 merged commit 97ed566 into elastic:master Dec 20, 2019
@flash1293 flash1293 deleted the data-apis branch December 20, 2019 18:05
flash1293 added a commit to flash1293/kibana that referenced this pull request Dec 20, 2019
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants