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

perf: use prettyPrint=false by default #28

Merged
merged 2 commits into from Sep 21, 2020

Conversation

tswast
Copy link
Contributor

@tswast tswast commented Sep 16, 2020

This tells One Platform APIs not to use excessive whitespace in
responses. For some reason, the backend defaults to prettyPrint=true and
does not provide a mechanism for overriding this.

In response to internal customer issue 168522192.

Tested locally against google-cloud-bigquery system and sample tests.

This tells One Platform APIs not to use excessive whitespace in
responses. For some reason, the backend defaults to prettyPrint=true and
does not provide a mechanism for overriding this.

In response to internal customer issue 168522192.

Tested locally against `google-cloud-bigquery` system and sample tests.
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 16, 2020
if query_params:
url += "?" + urlencode(query_params, doseq=True)

if "prettyPrint" not in query_params:

Choose a reason for hiding this comment

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

Trying to think of cases where this could alter requests we're not anticipating and may have issue with the prettyprint param.

  • will this interfere with media uploads? Mainly thinking about the resumable upload scenario.
  • will this interfere with oauth negotiation?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could make a pre-release and verify that the tests for other libs that depend on google-cloud-core still pass. (The pre-release can also be given to the customer to pin explicitly).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will this interfere with media uploads? Mainly thinking about the resumable upload scenario.

Since google-resumable-media uses "requests" directly, this shouldn't affect it.

I've tested these changes locally with the BigQuery system & sample tests, which all pass and do several load jobs.

I'm trying to test locally with the Storage system tests, but seem to be missing some setup steps around their encryption-related tests.

will this interfere with oauth negotiation?

No, since google-auth uses "requests" directly. It doesn't go through this library. Even if it did, I believe they've actually moved to a One Platform API for fetching tokens now. (Guessing based on the new-ish oauth2.googleapis.com endpoint)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a pre-release at #31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants