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

[Reporting] Remove boilerplate needed to get thegetScreenshots function #64269

Merged
merged 3 commits into from
Apr 27, 2020

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Apr 22, 2020

Summary

This PR removes the boilerplate needed for gaining a handle to the screenshot capture function. This is groundwork for moving the Reporting queue to Task Manager and ditching ESQueue.

This PR begins work on a future goal to allow outside plugins to generate their screenshots using the Reporting plugin pipeline. Once the Reporting code is fully migrated to the New Platform, exposing our screenshot capture function in the plugin contract would just be to change the ReportingSetup interface:

-export type ReportingSetup = object;
+export interface ReportingSetup {
+  getScreenshotsObservable: ReportingCore['getScreenshotsObservable'];
+}

...and return an object with that function from ReportingPlugin#start. Then an outside plugin could add reporting as a dependency in the kibana.json and would get the ReportingSetup object in their plugin's start function.

The first use case of using reporting as a dependency should be to build a performance-measuring tool that helps the developers make incremental improvements to make reports run faster. See a draft plugin at: https://github.com/tsullivan/awesome_reporting_tools and the integration code at https://github.com/tsullivan/awesome_reporting_tools (depends on changes that are not in this PR).

Checklist

Delete any items that are not applicable to this PR.

For maintainers

const generatePngObservable = generatePngObservableFactory(
captureConfig,
mockBrowserDriverFactory
);
Copy link
Member Author

Choose a reason for hiding this comment

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

lots of boilerplate getting simplified :)

@tsullivan tsullivan force-pushed the reporting/add-plugin-contract branch from 6defc0f to 3e4e482 Compare April 23, 2020 18:43
@tsullivan tsullivan changed the title [Reporting] Add a plugin contract that exposes getScreenshots [Reporting] Remove boilerplate needed to get thegetScreenshots function Apr 23, 2020
@tsullivan tsullivan marked this pull request as ready for review April 23, 2020 22:54
@tsullivan tsullivan added v7.8 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 and removed v7.8 labels Apr 23, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

Copy link
Contributor

@joelgriffith joelgriffith left a comment

Choose a reason for hiding this comment

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

LGTM!

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@poffdeluxe poffdeluxe left a comment

Choose a reason for hiding this comment

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

Nice. This is super clean!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@tsullivan tsullivan merged commit 54dc148 into elastic:master Apr 27, 2020
@tsullivan tsullivan deleted the reporting/add-plugin-contract branch April 27, 2020 21:25
tsullivan added a commit to tsullivan/kibana that referenced this pull request Apr 27, 2020
…tion (elastic#64269)

* expose reportingCore in ReportingSetup for apps

* improve tests

Co-authored-by: Elastic Machine <[email protected]>
tsullivan added a commit that referenced this pull request Apr 27, 2020
…tion (#64269) (#64591)

* expose reportingCore in ReportingSetup for apps

* improve tests

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
@sophiec20 sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants