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

Injected headers are casted to strings #288

Merged
merged 1 commit into from
Jun 12, 2017

Conversation

macisamuele
Copy link
Collaborator

As done in bravado-core PR #115 we should cast headers injected via _request_options to string.
This is a workaround for requests issue #3491. More context on bravado-core PR

@coveralls
Copy link

coveralls commented Jun 11, 2017

Coverage Status

Coverage remained the same at 97.967% when pulling c8686cb on macisamuele:handle-requests-issue-3491 into a526c35 on Yelp:master.

@macisamuele macisamuele merged commit 3ca98af into Yelp:master Jun 12, 2017
@macisamuele macisamuele deleted the handle-requests-issue-3491 branch June 12, 2017 08:16
@sjaensch
Copy link
Contributor

@macisamuele I think we're converting the header values too soon. We've got internal reports that this change breaks validation for non-string parameters that are passed in the header. I think we need to do the stringifying in bravado-core.

@macisamuele
Copy link
Collaborator Author

@sjaensch ... that's weird to me ... are we able to reproduce the issue in order to write a test that targets it?
We can "easily" moving the conversion to string of the headers just before the return statement (https://github.com/Yelp/bravado/pull/288/files#diff-b7921701c1024606d23a5ebb67b3a930R297)

macisamuele added a commit that referenced this pull request Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants