Skip to content

Added a factory method that supports BASIC auth.#12

Open
carlossg wants to merge 1 commit intowillroden:masterfrom
cloudbees:basic
Open

Added a factory method that supports BASIC auth.#12
carlossg wants to merge 1 commit intowillroden:masterfrom
cloudbees:basic

Conversation

@carlossg
Copy link

@carlossg carlossg commented Aug 4, 2015

My original requirement is to be able to send an HTTP basic
authentication along with requests.

I took the liberty to also add a version of a method that takes
arbitrary RequestInterceptors to support other possible modes of
authentication, and general control. I think it's worthwhile doing this
now to avoid ending up with 20 different overloaded flavors of the
getInstance() method, but I can imagine you might not want to expose
Feign or perhaps go the other way and not have the
getInstanceWithBasicAuth method.

My original requirement is to be able to send an HTTP basic
authentication along with requests.

I took the liberty to also add a version of a method that takes
arbitrary RequestInterceptors to support other possible modes of
authentication, and general control. I think it's worthwhile doing this
now to avoid ending up with 20 different overloaded flavors of the
getInstance() method, but I can imagine you might not want to expose
Feign or perhaps go the other way and not have the
getInstanceWithBasicAuth method.
@carlossg
Copy link
Author

carlossg commented Aug 4, 2015

Related to #8

@WillAbides
Copy link
Member

I think I like this way of handling it, but I want to sleep on it before I merge.

@seh
Copy link

seh commented Aug 5, 2015

Per @carlossg's comment regarding #8, this approach is a gloss layer atop what's already afforded by a715a40. A caller could take care of setting up the RequestInterceptor for authentication. Helping a caller do so is something to consider.

Copy link

Choose a reason for hiding this comment

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

requestInterceptors overwrites existing requestInterceptors, so the MarathonHeadersInterceptor gets removed in the process.

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.

5 participants