Switch all PDF render traffic to new Proton service

[hknustwmf]

Removed old pdf.js module that split traffic between electron and proton. Changed configs to route traffic to proton PDF rendering only.

Bug: T210651

[Pchelolo]

Hm. Seems like travis is not working anymore, and it's unrelated to this PR. Could you please figure it out in a separate PR? I believe something like this should work https://github.com/rubygems/rubygems.org/pull/1963/files

[Pchelolo]

There are 2 unit tests for this in /test/features/pdf.js that were ignored because the old renderer was very unreliable. We should bring them back.

[hknustwmf]

Typo. Already fixed. Also removed the lock file that went in because of it

…

On Mon, Apr 29, 2019 at 11:36 AM Petr Pchelko ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In .gitignore <#1126 (comment)>: > @@ -3,11 +3,15 @@ coverage config.yaml node_modules npm-debug.log +package-json.lock package-lock.json you mean? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1126 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALEHXVA6PZ4WCKJNWE6IKKDPS4IYTANCNFSM4HJEHKKQ> .

-- Holger Knust Software Engineer, Core Platform Team Wikimedia Foundation <https://wikimediafoundation.org/>

[Pchelolo]

Please rebase against #1127

[Pchelolo]

How did the commit 985d008 got in here? :)

[d00rman]

Looking good! Now we also need to add the extra parameters that are supported by the Proton service, but were not supported in Electron, just as it was done in PR #1090. Note, though, that in that PR we still use the optional param syntax, which we have since dropped. So, there should be three end points now: /pdf/{title}, /pdf/{title}/{format} and /pdf/{title}/{format}/{type}. The default values, if not supplied, should be format: a4 and type: desktop.

[Pchelolo]

Looking good! Now we also need to add the extra parameters that are supported by the Proton service, but were not supported in Electron, just as it was done in PR #1090. Note, though, that in that PR we still use the optional param syntax, which we have since dropped. So, there should be three end points now: /pdf/{title}, /pdf/{title}/{format} and /pdf/{title}/{format}/{type}. The default values, if not supplied, should be format: a4 and type: desktop.

nah @d00rman we ate not doing this yet. my idea is to have the public API intact now untill we are completely sure we are switching to the extended functionality. thus the hard-coded formats. It would be good to write a comment about that though

[hknustwmf]

Add comment to pdf.yaml about the hard-coded parameters to be replaced by endpoint parameters

[d00rman]

@hknustwmf please address the timeout issue in tests

[d00rman]

Nice work @hknustwmf !

[d00rman]

@hknustwmf this now needs a manual rebase

[d00rman]

Un-approving until the rebase happens

[d00rman]

Still needs a rebase, somehow

[hknustwmf]

Still needs a rebase, somehow

Do you want me to combine the commits into 1?

[d00rman]

Alrighty, this is looking good now. We still need to hold off on merging until the back-end is fully ready for the transition.

[d00rman]

One more thing: let's implement rate limiting given the resources needed to generate PDFs. To do that, you need to add the rate-limiting route filter under the x-route-filters stanza:

x-route-filters:
  - path: ./lib/access_check_filter.js
    options:
      redirect_cache_control: '{{options.cache_control}}'
  - type: default
    name: ratelimit_route
    options:
      limits:
        internal: 10
        external: 5

[d00rman]

Now we are solid, thnx @hknustwmf !

[d00rman]

Actually, @hknustwmf let's correct the rate-limiting values. Let's set them to 1 for external requests and 5 for internal ones. The limits are per-client, so let's lower them to the minimum.

[hknustwmf]

Actually, @hknustwmf let's correct the rate-limiting values. Let's set them to 1 for external requests and 5 for internal ones. The limits are per-client, so let's lower them to the minimum.

Just pushed the changes.