Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEMP! Split PDF traffic #1043

Merged
merged 3 commits into from
Sep 18, 2018
Merged

TEMP! Split PDF traffic #1043

merged 3 commits into from
Sep 18, 2018

Conversation

Pchelolo
Copy link
Contributor

@Pchelolo Pchelolo commented Aug 2, 2018

@Pchelolo Pchelolo requested a review from d00rman August 2, 2018 19:17
@Pchelolo
Copy link
Contributor Author

Pchelolo commented Aug 2, 2018

It's a no-op until a config change. We can make the config change in beta cluster only for now.

On a later stage we will expose the type and format parameters as well - as soon as we're sure we're switching to the new imlementation.

@Pchelolo
Copy link
Contributor Author

Pchelolo commented Aug 2, 2018

The tests pass locally... interesting

Copy link
Contributor

@d00rman d00rman left a comment

Choose a reason for hiding this comment

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

LGTM. We should also remember to make a grafana dashboard for Proton.

@d00rman
Copy link
Contributor

d00rman commented Aug 3, 2018

We should also remember to make a grafana dashboard for Proton.

Created T201158 for that. Let's not merge this PR until that is completed.

@d00rman
Copy link
Contributor

d00rman commented Aug 28, 2018

@Pchelolo could we also have an undocumented query param that allows one to retrieve the PDF generated by Proton? I figure this would be very helpful for Reading to evaluate the visual output that it generates.

@Pchelolo
Copy link
Contributor Author

could we also have an undocumented query param that allows one to retrieve the PDF generated by Proton?

Done, added new_pdf hidden query parameter.

@d00rman
Copy link
Contributor

d00rman commented Aug 29, 2018

Done, added new_pdf hidden query parameter.

Obrigado @Pchelolo !

Change-Id: I2cd0166620c9933e37c600af5b65ced4fd7009b5
Change-Id: Ieb8ca86a67ece346273a02240889482c5e365be0
Change-Id: Idf03eef3feefa82aa308d4f674870ec4d05f15e0
@d00rman d00rman added the API label Sep 18, 2018
@d00rman d00rman merged commit d2623ce into wikimedia:master Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants