Skip to content

[Logs UI / Metrics UI] New Platform migration server followups#51615

Merged
Kerry350 merged 103 commits intoelastic:masterfrom
Kerry350:np-migration-followups
Dec 17, 2019
Merged

[Logs UI / Metrics UI] New Platform migration server followups#51615
Kerry350 merged 103 commits intoelastic:masterfrom
Kerry350:np-migration-followups

Conversation

@Kerry350
Copy link
Copy Markdown
Contributor

@Kerry350 Kerry350 commented Nov 25, 2019 •
edited
Loading

⚠️ ⚠️ ⚠️ This contains all of the commits of #45299 and shouldn't be reviewed until that is merged

Summary

This is currently a WIP, but open for visibility.

This is a followup PR to #45299, and contains changes that were important, but not blockers in getting the other PR merged.

  • Adds types for plugin dependencies where possible (0a40478)
  • Fully converts Metrics Explorer to using io-ts. This reinstates validation for the Metrics Explorer API which was temporarily disabled (6748dab).
  • Exposes our public contract, for other plugins to consume, via the setup() method return statement (a918f08) (This isn't used yet, but it just gets us ready for when we move 100% to the NP world / directory).
  • Adds a kibanaVersion property to the kibana.json manifest for the Infra plugin (b9cb4ee).

The changeset here looks a lot bigger than it realistically is. The vast majority of the changes relate to the io-ts conversion for Metrics Explorer. As some of the enums have changed to string unions a lot of files were touched, but for minimal changes.

For testing this PR, please ensure the Metrics portions of the UI still work as expected.

(@elastic/apm-ui have been automatically requested for review as I've removed a comment from an APM file that we left behind in the big migration PR.)

jasonrhodes and others added 30 commits October 23, 2019 13:39
@jasonrhodes
Basic cleanup before refactoring for shim work
735f57e
@jasonrhodes
shim WIP
d46a6ab
@jasonrhodes
Removes the configuration adapter
6407924
@jasonrhodes
WIP more stuff
51fc360
@jasonrhodes
WIP refactoring of shimming work
dd83941
@jasonrhodes
WIP continues
0ad3df3
@jasonrhodes
Logging UI now runs on top of new platform shim
c5eb8cc
@jasonrhodes
WIP continues
b9413b2
@jasonrhodes
Removes unused imports and variables
cda2d41
@jasonrhodes
Basic infra NP server shim in place
49b0d7e
@jasonrhodes
Reimplemented graphql http error handling for infra NP server shim
1ef3408
@jasonrhodes
Adds new platform infra plugin to handle NP config for legacy server …
9d01529 
…shim
@jasonrhodes
Basic cleanup before refactoring for shim work
daea0e6
@jasonrhodes
shim WIP
3cacf78
@jasonrhodes
Removes the configuration adapter
188e947
@jasonrhodes
WIP more stuff
222d7e5
@jasonrhodes
WIP refactoring of shimming work
f599939
@jasonrhodes
WIP continues
b594898
@jasonrhodes
Logging UI now runs on top of new platform shim
8502516
@jasonrhodes
WIP continues
51b5e8f
@jasonrhodes
Removes unused imports and variables
e12cbbd
@jasonrhodes
Basic infra NP server shim in place
b07fcf3
@jasonrhodes
Reimplemented graphql http error handling for infra NP server shim
bdc1c31
@jasonrhodes
Adds new platform infra plugin to handle NP config for legacy server …
a73b778 
…shim
@jasonrhodes
Adds comment about duplicating full config for NP config
8e00bfd
@Kerry350
Use New Platform features plugin to registerFeature()
42cf3e5
@jasonrhodes
Fixing merge conflicts from collaboration
f3bd21d
@jasonrhodes
Re-arranging and relying on request context as uch as possible
9982b9f
@jasonrhodes
Refactors KibanaRequest for RequestHandlerContext
4b7a649
@jasonrhodes
fixes types for callWithRequest
645fa97
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 6, 2019

💔 Build Failed

@Kerry350 Kerry350 requested a review from a team December 9, 2019 12:25
@Kerry350 Kerry350 added v7.6.0 v8.0.0 Feature:Logs UI Logs UI feature Feature:Metrics UI Metrics UI feature release_note:skip Skip the PR/issue when compiling release notes labels Dec 9, 2019
@Kerry350 Kerry350 marked this pull request as ready for review December 9, 2019 12:29
@Kerry350 Kerry350 requested a review from a team December 9, 2019 12:29
@Kerry350 Kerry350 mentioned this pull request Dec 9, 2019
Merged
@Kerry350 Kerry350 mentioned this pull request Dec 11, 2019
Merged
simianhacker
simianhacker requested changes Dec 11, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

Everything looks good. The only thing that stuck out to me is around the aggregation type. It seems like we are defining that list several times so I added some suggestions to try and reduce that some. If you want to just move forward with what you have, just submit for review again and I will approve.

jasonrhodes
jasonrhodes approved these changes Dec 12, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

I like Chris's dedupe suggestions but otherwise this LGTM

jasonrhodes
jasonrhodes reviewed Dec 12, 2019
View reviewed changes
x-pack/legacy/plugins/apm/server/lib/settings/apm_indices/get_apm_indices.ts
};
}

// export async function getApmIndices(context: APMRequestHandlerContext) {
Copy link
Copy Markdown
Member

@jasonrhodes jasonrhodes Dec 12, 2019

Choose a reason for hiding this comment

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

lol sorry, I think this was my fault -- thanks for the clean up :) <3

@Kerry350
Copy link
Copy Markdown
Contributor Author

Kerry350 commented Dec 16, 2019

@simianhacker Thanks for the review and suggestions. I was going to do these in a followup, but they didn't take very long so I've added them in 2139171. If you're happy and can approve I'll merge this 👍

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 16, 2019

💚 Build Succeeded

History

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

@simianhacker simianhacker self-requested a review December 17, 2019 17:57
simianhacker
simianhacker approved these changes Dec 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

LGTM

@Kerry350 Kerry350 merged commit 18b5cf9 into elastic:master Dec 17, 2019
@Kerry350 Kerry350 mentioned this pull request Dec 17, 2019
Merged
Kerry350 added a commit to Kerry350/kibana that referenced this pull request Dec 17, 2019
@Kerry350
[Logs UI / Metrics UI] New Platform migration server followups (elast…
8153daa 
…ic#51615)
Kerry350 added a commit that referenced this pull request Dec 17, 2019
@Kerry350
[Logs UI / Metrics UI] New Platform migration server followups (#51615)…
f86ae6c 
… (#53320)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasonrhodes jasonrhodes jasonrhodes approved these changes

@simianhacker simianhacker simianhacker approved these changes

+1 more reviewer

@sorenlouv sorenlouv sorenlouv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Kerry350 Kerry350

Labels

Feature:Logs UI Logs UI feature Feature:Metrics UI Metrics UI feature release_note:skip Skip the PR/issue when compiling release notes v7.6.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Kerry350 @elasticmachine @simianhacker @jasonrhodes @sorenlouv