Fix PDF request summary #6257
Closed
Fix PDF request summary #6257
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes the wkhtmltopdf command less verbose which is important as we check for output to determine if the command was successful or not. Really should be checking the exit code but this would mean a change in the AlaveteliExternalCommand module which is also used else where.
garethrees
reviewed
Aug 2, 2021
| @@ -678,13 +678,18 @@ def make_request_summary_file(info_request) | |||
| tmp_input.write(html_output) | |||
| tmp_input.close | |||
| tmp_output = Tempfile.new('foihtml2pdf-output') | |||
| output = AlaveteliExternalCommand.run(convert_command, tmp_input.path, tmp_output.path) | |||
There was a problem hiding this comment.
Think we could make this work by changing the convert_command to preserve the interface here.
# config/general.yml
HTML_TO_PDF_COMMAND: "/usr/local/bin/wkhtmltopdf -q"If this works then we'll want to update config/general.yml-example so that it gets set like this for everyone, and also note in the changelog that this is needed.
There was a problem hiding this comment.
This won't work without additional changes as this would eval to false:
convert_command = AlaveteliConfiguration.html_to_pdf_command
File.exist?(convert_command)There was a problem hiding this comment.
How about something like:
# Default
HTML_TO_PDF_COMMAND: "/usr/local/bin/wkhtmltopdf"
# Optional
HTML_TO_PDF_COMMAND: ["/usr/local/bin/wkhtmltopdf", "-q"]convert_command = Array(AlaveteliConfiguration.html_to_pdf_command)
cmd = convert_command.shift
File.exist?(cmd)
output = AlaveteliExternalCommand.run(cmd, convert_command, tmp_input.path, tmp_output.path)
This was referenced Aug 4, 2021
|
Superseded by #6488? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relevant issue(s)
Connected to #6056
What does this do?
Add
-qoption to calls towkhtmltopdfThis makes the
wkhtmltopdfcommand less verbose which is important as we check for output to determine if the command was successful or not.Really should be checking the exit code but this would mean a change in the
AlaveteliExternalCommandmodule which is also used else where.Why was this needed?
This is broken in production and only
txtcorrespondence is being downloaded. Not sure when this broken possiblly related to https://github.com/mysociety/sysadmin/issues/867