Skip to content

Conversation

@vam-google
Copy link
Contributor

@vam-google vam-google commented May 17, 2021

Also add Content-Type: application/json header.

This PR depends on googleapis/python-api-core#189, and assumes that google-api-core version 1.27.0 has been already released (which was not the case on the moment of creation of this PR).

Also add `Content-Type: application/json` header
@vam-google vam-google requested a review from a team as a code owner May 17, 2021 09:50
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 17, 2021
@codecov
Copy link

codecov bot commented May 17, 2021

Codecov Report

Merging #888 (870124f) into master (7c185e8) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #888   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        27    +1     
  Lines         1608      1697   +89     
  Branches       328       347   +19     
=========================================
+ Hits          1608      1697   +89     
Impacted Files Coverage Δ
gapic/samplegen_utils/types.py 100.00% <ø> (ø)
gapic/samplegen_utils/utils.py 100.00% <ø> (ø)
gapic/utils/case.py 100.00% <ø> (ø)
gapic/utils/reserved_names.py 100.00% <ø> (ø)
gapic/generator/generator.py 100.00% <100.00%> (ø)
gapic/samplegen/samplegen.py 100.00% <100.00%> (ø)
gapic/schema/api.py 100.00% <100.00%> (ø)
gapic/schema/metadata.py 100.00% <100.00%> (ø)
gapic/schema/wrappers.py 100.00% <100.00%> (ø)
gapic/utils/__init__.py 100.00% <100.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce558ac...870124f. Read the comment docs.


# Send the request
headers = dict(metadata)
headers['Content-Type'] = 'application/json'
Copy link
Contributor

Choose a reason for hiding this comment

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

Naive question: this is not x-goog-api-client. Where does the name content-type come from? Is there documentation somewhere describing it?

Copy link
Contributor Author

@vam-google vam-google May 18, 2021

Choose a reason for hiding this comment

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

The Content-Type header is a part of HTTP standard: https://datatracker.ietf.org/doc/html/rfc2616#section-14.17

Copy link
Contributor

Choose a reason for hiding this comment

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

@software-dov See also the internal REGAPIC gRPC transcoding design doc, where we call this out explicitly (relatively recent addition, since we found some RPCs fail without it)

@vam-google
Copy link
Contributor Author

@busunkim96 PTAL

Copy link
Contributor

@busunkim96 busunkim96 left a comment

Choose a reason for hiding this comment

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

LGTM.

I suspect the coverage drop is from the clauses I added to keep compatibility with older google-auth and google-api-core versions in 89d6f35#diff-f7a16a65f061822bcc73b8296f4dc837353d379d8d9cc5307982cb6941442835.

@busunkim96
Copy link
Contributor

Opened PR to remove code / tests for google-api-core < 1.26.0. See #893

Once that is merged updating this branch should resolve the coverage issues.

@busunkim96
Copy link
Contributor

@vam-google CI is now passing.

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.

4 participants