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

Allow to set additional Http headers directly on the IClientExecutable #1152

Merged
merged 1 commit into from
May 30, 2019

Conversation

ohr
Copy link
Collaborator

@ohr ohr commented Dec 19, 2018

I'd like to propose an extension to the IClientExecutable interface that allows to set arbitrary HTTP headers. This is an alternative to using AdditionalRequestHeadersInterceptor, with one important difference: it makes the client stateless so it can even be reused with header values like OAuth2 tokens that change with every request.
And I think it's simply more straightforward to use.

@jamesagnew
Copy link
Collaborator

Is there a reason you wouldn't just use AdditionalRequestHeadersInterceptor to add arbitrary headers?

@ohr
Copy link
Collaborator Author

ohr commented Jan 21, 2019

It's too static - the interceptor has predefined header/value pairs. interceptRequest(IHttpRequest theRequest) allows no further client-side context (like information of the user for whom the request is executed).
This forces me to either create a new IRestfulClient for every request or to use Thread-Local variables; and both is not nice for the simple task of adding a header.
Also, the majority of IClientInterceptor implementations deal with adding HTTP headers (all suffer from the same restrictions I mentioned above), so a more straightforward API seems justified for me.

@jamesagnew jamesagnew merged commit cd8b2fe into hapifhir:master May 30, 2019
jamesagnew added a commit that referenced this pull request May 30, 2019
@ohr ohr deleted the client-setheader branch May 31, 2019 07:36
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.

2 participants