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

vis_type_timeseries server side new platform migration #52501

Merged
merged 119 commits into from
Dec 19, 2019

Conversation

Kerry350
Copy link
Contributor

@Kerry350 Kerry350 commented Dec 9, 2019

⚠️ ⚠️ ⚠️ This currently contains the commits from #51615 and shouldn't be reviewed until that's merged.

Summary

WIP of the migration of the vis_type_timeseries plugin to the new platform, on the server side.

This is a server side migration of the vis_type_timeseries plugin to the new platform. It migrates what was needed for the Logs and Metrics dependencies.

What's covered?

  • The vis_type_timeseries plugin now has a New Platform plugin, within the src/plugins directory.
  • The NP plugin now exports getVisData as part of it's public contract to other plugins within the setup() method.
  • The Infra plugin has been changed to use the NP version of the vis_type_timeseries ("metrics") plugin, with all hacks / workarounds removed. Infra was the only plugin I could find using the getVisData function in a plugin <-> plugin capacity.
  • The getVisData function has a facade added at the top level to augment an object to look like the old req it required, but using the new requestContext and request from the New Platform router. This was to make refactoring easier at this stage, there are no TypeScript types to aid in changing this code, so changing many layers of code seemed brittle.
  • The /api/metrics/vis/data API endpoint has been fully migrated to the NP router. This is the endpoint things like Kibana's Visualize uses.

What's not covered?

  • The fields and search strategies routes have not been migrated to the NP router. They still register their routes via ___LEGACY.server.

@Kerry350 Kerry350 self-assigned this Dec 9, 2019
@Kerry350 Kerry350 added release_note:skip Skip the PR/issue when compiling release notes v7.6.0 v8.0.0 labels Dec 18, 2019
@Kerry350 Kerry350 marked this pull request as ready for review December 18, 2019 12:40
@Kerry350 Kerry350 requested a review from a team as a code owner December 18, 2019 12:40
Copy link
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.

LGTM, legacy server registration is still a little hard to follow but moving init directly into the register legacy API method makes it much much easier than the observable flow before, and I'm not sure we can do better until things are migrated. Thanks so much for taking this on!!!

@@ -25,7 +25,7 @@ describe('getEsShardTimeout', () => {
const req = {
getEsShardTimeout: async () => {
return 12345;
}
},
Copy link
Member

Choose a reason for hiding this comment

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

ugh lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know right...dunno why it didn't tell me in the first place 🤔

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants