Skip to content

Switch back to using syncronous pdf export#275

Open
TheSyscall wants to merge 2 commits intomainfrom
sync-pdf-export
Open

Switch back to using syncronous pdf export#275
TheSyscall wants to merge 2 commits intomainfrom
sync-pdf-export

Conversation

@TheSyscall
Copy link
Copy Markdown

@TheSyscall TheSyscall commented Apr 8, 2026

The reason we initially switched to asyncHtmlToPdf was because the headless chrome backend tried to create its own event loop, which caused problems with ipl-scheduler.

The new pdfexport module no longer has this behavior. Therefore, asyncHtmlToPdf is no longer required.
The method was never actually part of the hook and therefore relied on the fact that pdfexport was the only implementation of PdfexportHook.

requires Icinga/icingaweb2#5491
requires Icinga/icingaweb2#5480
requires Icinga/icingaweb2-module-pdfexport#88

The reason we initially switched to asyncHtmlToPdf was because the
headless chrome backend tried to create its own event loop, which caused
problems with ipl-scheduler.

The new pdfexport module no longer has this behaviour.
Therefore asyncHtmlToPdf is no longer required.
The method was never actually part of the hook and therefore relied on
the fact that pdfexport was the only implementation of PdfexportHook.
Copy link
Copy Markdown
Contributor

@sukhwinder33445 sukhwinder33445 left a comment

Choose a reason for hiding this comment

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

Please adjust the requirements. This PR requires Icinga/icingaweb2#5491, Icinga/icingaweb2#5480 and Icinga/icingaweb2-module-pdfexport#88.

While testing, I noticed that BackendLocator::getFirstSupportedBackend() (introduced with Icinga/icingaweb2-module-pdfexport#88) is called twice, once in Pdfexport::isSupported() and once in Pdfexport::htmlToPdf(). This seems redundant and results in duplicate log entries.

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants