Skip to content
This repository has been archived by the owner on Apr 14, 2024. It is now read-only.

Allow "guzzlhttp/psr7": "^2" #28

Closed
florentdestremau opened this issue Sep 15, 2022 · 6 comments · May be fixed by #29
Closed

Allow "guzzlhttp/psr7": "^2" #28

florentdestremau opened this issue Sep 15, 2022 · 6 comments · May be fixed by #29

Comments

@florentdestremau
Copy link
Contributor

Current Behavior

I have deprecation warnings because guzzlhttp/psr7 (a dependency of this project) is still in v1. I would like to use v2, but the version constraints don't allow this.

Possible Solution

Loosen the requirements from "guzzlehttp/psr7": "^1.4.2" to guzzlehttp/psr7": "^1.4.2|^2.0

@gulien
Copy link
Contributor

gulien commented Sep 15, 2022

Hello @florentdestremau,

I do not maintain this repository anymore. Please consider using Gotenberg 7 and https://github.com/gotenberg/gotenberg-php.

You may also fork this repository.

@gulien gulien closed this as completed Sep 15, 2022
@florentdestremau
Copy link
Contributor Author

Hi 👋

Does the new repo allow the usage for Gotenberg 6.x ? I spent a whole day trying to transition to v7 (docker + php lib) and was furstrated: more timeouts in rendering pdfs, less performance... so I'll stick to 6.* for now.

I did fork this repo but I think this could help out other people using this lib like me.

florentdestremau added a commit to florentdestremau/gotenberg-php-client that referenced this issue Sep 15, 2022
As stated here thecodingmachine#28 (comment) I think it's useful to be explicit about it 🙂
@gulien
Copy link
Contributor

gulien commented Sep 15, 2022

Thanks for the feedback!

Out of curiosity, what's your use case? I wonder what performances issues you were encountering.

@florentdestremau
Copy link
Contributor Author

We generate PDF reports from surveys made on our platform. A couple of pages with graphs, some CSS, etc..

We've been pulling our hair out over performance issues during our migration to a new hosting system. I think it's related to several things:

  • we use google fonts and they are not retrieved by the new host (dns problem probably, but impossible to have clear errors in the logs)
  • we host images on a cdn, and it is not retrieved as well,so we have broken images on our reports.

So at the end of the day it looks like a big part of our performance issue was network (we can't find ou why tho).

One other test we did was simply try to convert google.com in pdf, and on 6.3.1 it takes about 2s, while on 7.x it takes 4 on the same server. That's how we could quantify a performance gap between the 2 versions. The DNS errors added up on top of that.

To switch we'll need to import fonts I think and import the cdn assets into the project source code so that we can push them directly with the html.

@gulien
Copy link
Contributor

gulien commented Sep 15, 2022

👍

we use google fonts and they are not retrieved by the new host
we host images on a cdn

It should work out of the box. Still, I've tested converting this HTML on the demo instance, there was 0 issue with the Google font and the "remote" image.

As you said, it looks mostly as an issue on your network than Gotenberg itself.

One other test we did was simply try to convert google.com in pdf, and on 6.3.1 it takes about 2s, while on 7.x it takes 4 on the same server.

That does actually make sense. Gotenberg 6 uses a long-running Chrome instance, while Gotenberg 7 start a dedicated instance per conversion.

There is definitely a performance drawback per conversion, but:

  • It allows for more concurrent conversions (and therefore quicker batch conversions).
  • There are no more memory leaks with Chrome.

Another performance drawback comes from the fact that Gotenberg 7 checks every requests that a page does. This is mostly a security requirement, because in Gotenberg 6 one could print the content of the /etc/passwd file for instance (got some CVE because of this).

That being said, I could add an option to skip this check for users that trust the inputs sent to Gotenberg.

@florentdestremau
Copy link
Contributor Author

Interesting, we never had problems with memory leaks (that I know of), and most of our conversions are done in batch on a 4-workers async queue so not a lot of concurrency at the moment, but I can see why it would help.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants