Skip to content

Conversation

@yang-g
Copy link
Contributor

@yang-g yang-g commented Aug 26, 2016

As upper-case letters are illegal per http2 spec.

As upper-case letters are illegal per http2 spec.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 26, 2016
@yang-g
Copy link
Contributor Author

yang-g commented Aug 27, 2016

#2204

@tseaver tseaver added the grpc label Aug 27, 2016
@tseaver
Copy link
Contributor

tseaver commented Aug 27, 2016

@yang-g Thanks for the fix! Travis is going to barf whenever it gets around to running, because the unit tests assert the upper-case field names:

======================================================================
FAIL: test___call__ (gcloud.test__helpers.TestMetadataPlugin)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tseaver/projects/agendaless/Google/src/gcloud-python/gcloud/test__helpers.py", line 845, in test___call__
    self.assertEqual(callback_args, [(cb_headers, None)])
AssertionError: Lists differ: [([('authorization', 'Bearer F... != [([('Authorization', 'Bearer F...

First differing element 0:
([('authorization', 'Bearer FOOBARBAZ'), ('user-agent', 'USER_AGENT')], None)
([('Authorization', 'Bearer FOOBARBAZ'), ('User-agent', 'USER_AGENT')], None)

- [([('authorization', 'Bearer FOOBARBAZ'), ('user-agent', 'USER_AGENT')], None)]
?      ^                                      ^

+ [([('Authorization', 'Bearer FOOBARBAZ'), ('User-agent', 'USER_AGENT')], None)]
?      ^                                      ^

Can you please update the PR accordingly?

@dhermes, @daspecster For future reference, here is the HTTP/2 spec reference which mandates lower-case for header field names (I was completely unaware of that requirement).

@tseaver
Copy link
Contributor

tseaver commented Aug 27, 2016

I cancelled the not-yet-started Travis build after verifying that it would fail.

@yang-g
Copy link
Contributor Author

yang-g commented Aug 27, 2016

@tseaver I pushed eebb83a about the same time of your comment. I did a search and did not find anything else. Could you restart the travis-ci or point me to more places to fix? Thanks.

@tseaver
Copy link
Contributor

tseaver commented Aug 27, 2016

@yang-g I restarted the build. Sorry for my haste in cancelling -- Travis has been so glacially slow lately that we've been minimizing "known broken" jobs in the queue.

@dhermes
Copy link
Contributor

dhermes commented Aug 27, 2016

@tseaver I think Travis have sorted out their issues

@tseaver tseaver merged commit a5594a7 into googleapis:master Aug 27, 2016
@tseaver
Copy link
Contributor

tseaver commented Aug 27, 2016

@yang-g Thank you again!

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. grpc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants