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 ] run task function simplifications of image export types #174410

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Jan 6, 2024

Summary

This PR cleans up extra layers of abstraction in image export types that could complicate progress of the proposal of "auto" timeouts. See #131852

Changes

Minor code cleanup of the "runTask" functions of export types that create screenshots, by removing the generatePdf* / generatePng helper callback functions and inlining the work those modules were doing.

The helper modules were an integral part of the screenshotting pipelines, but in the unit tests they were completely mocked. Now that we have a proper mock utility of the screenshotting plugin start contract, we no longer need mockable code in a separate layer of the pipelines.

@tsullivan tsullivan force-pushed the reporting-image-export-cleanups branch from ae688e9 to bb06f98 Compare January 9, 2024 23:23
@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Jan 9, 2024
@tsullivan tsullivan marked this pull request as ready for review January 9, 2024 23:28
@tsullivan tsullivan requested a review from a team as a code owner January 9, 2024 23:28
@tsullivan tsullivan changed the title [Reporting ] run task fn simplifications [Reporting ] run task function simplifications of image export types Jan 9, 2024
@tsullivan
Copy link
Member Author

buildkite build this

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

Wooohooo getting rid of pdf_v1 and png! Yay!

@tsullivan tsullivan merged commit 5a8b7af into elastic:main Jan 10, 2024
20 checks passed
@tsullivan tsullivan deleted the reporting-image-export-cleanups branch January 10, 2024 16:58
delanni pushed a commit to delanni/kibana that referenced this pull request Jan 11, 2024
…lastic#174410)

## Summary

This PR cleans up extra layers of abstraction in image export types that
could complicate progress of the proposal of "auto" timeouts. See
elastic#131852

## Changes
Minor code cleanup of the "runTask" functions of export types that
create screenshots, by removing the `generatePdf*` / `generatePng`
helper callback functions and inlining the work those modules were
doing.

The helper modules were an integral part of the screenshotting
pipelines, but in the unit tests they were completely mocked. Now that
we have a proper mock utility of the `screenshotting` plugin start
contract, we no longer need mockable code in a separate layer of the
pipelines.

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants