Generate PDFs with chromium, remove wicked_pdf #453
Conversation
wioux
changed the title
| @@ -4,7 +4,6 @@ namespace :pdfs do | |||
| task recreate: :environment do | |||
| Lesson.published.find_each do |lesson| | |||
| UpdateLessonPdf.perform_now(lesson.id) | |||
| sleep 0.5 | |||
spec/lib/pdf_template_spec.rb
Outdated
| at_least(:once) | ||
|
|
||
| html = %(<a href="/relative">anchor</a>") | ||
| expect(pdf_template.send(:rebase_urls, html)). |
There was a problem hiding this comment.
why is it necessary to send :rebase_urls?
There was a problem hiding this comment.
In that case, maybe consider finding some other way to test? One of the only purposes of making a method private is to tell coders that it's not a reliable feature of the object's public interface. The internet is divided over this, but that's how I was taught.
|
@vbrown608 For now I'd like to take the |
lib/pdf_template.rb
Outdated
| def print_pdf(input) | ||
| output = Tempfile.new(["pdf", ".pdf"]) | ||
| chrome_wrapper = Rails.root.join("bin/chrome-stub").to_s | ||
| command = ["timeout", "-t", "5", |
There was a problem hiding this comment.
Pls use Ruby's timeout instead, for cross-platform compatibility!
| @@ -28,6 +29,7 @@ RUN echo "@edge http://nl.alpinelinux.org/alpine/edge/main" >>/etc/apk/repositor | |||
|
|
|||
There was a problem hiding this comment.
Can we remove xvfb and wkhtmltopdf@edgetesting from the Dockerfile?
|
|
||
| #main-content { | ||
| margin-top: 65px; | ||
| } |
There was a problem hiding this comment.
Some styles aren't getting picked up when I test this locally and the image doesn't seem to be coming through. The font size (or maybe weight?) also seems a bit heavy. I'm not sure if this is unique to my setup, maybe we can check in about it on Monday.
There was a problem hiding this comment.
I pushed a change to inline the stylesheet and use a data URI for the header image, which should be more reliable.
|
This is ready for review again. I switched to a different node library, which doesn't have the problem with stalling jobs (though I'm keeping it wrapped in a timeout block just in case). It also has better support for printing from an existing/remote chrome process (instead of spawning a new one each time) so we should be able to follow up soon with moving the browser to its own container. |
wkhtmltopdf has more limitations and bugs (links in our PDFs are not clickable!), and we also have issues with having to keep an xvfb display running in our container.
As mentioned in #447 ideally we would have chrome running in a separate container. Right now the
chrome-headless-render-pdfpackage I'm using doesn't support connecting to a remote instance but it looks like an easy-ish patch we could make. I intend to follow up on it but meanwhile I think we are somewhat better off running chrome headless rather than xvfb + wicked_pdf.Depends on #450