-
Notifications
You must be signed in to change notification settings - Fork 731
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
Dynamic configuration of http headers #289
Conversation
@viniciussoares: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
Can we get this PR merged as a temporary fix even if there are plan for longer term fix? |
@amol-c as a temporary workaround update your Podfile to use my repository.
|
|
||
|
||
httpHeaders?.forEach { header in | ||
request.addValue(header.value, forHTTPHeaderField: header.key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have 2 extra spaces before request
for consistent indentation
Could we instead get a delegate? This would make per-request headers much easier (ie per request signing) and make it more extensible to other changes or instrumentation for any part of the http request before it is fired off. |
I agree with @brennantaylor that adding a property for headers won't be flexible enough to support the use cases that have come up. I added some more detail about this in #297 (comment). |
I agree :) Closed in favor of #297 |
@viniciussoares Thanks, your PR inspired me to get off my butt and open one of my own :p |
Allow http headers to be reconfigured through HTTPNetworkTransport.
This will probably resolve the following issues:
#37
#140
#131 (Never been merged)
/enhancement