-
Notifications
You must be signed in to change notification settings - Fork 133
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
Add support for java Proxy with basic auth #266
Conversation
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.
Just some minor feedback, mostly around JavaDocs. Otherwise this looks good.
Regarding the names, I think ClientOptions
and ProxyOptions
are appropriate. Were there other names you were considering that you want to raise for discussion?
One other question I have is if/how you tested the proxy support (aside from the unit tests that rely on mocks)?
Changes
This PR adds a new constructor for both
AuthAPI
andManagementAPI
classes that accepts a set ofClientOptions
(final name can still change).The idea is that from now on, every new property that we want to configure in the OkHttpClient comes through this options object. This way, we reduce the exposure of third-party classes in our public API.
For now, this Options object accepts a
ProxyOptions
object (final name also subject to change), that will hold the Proxy instance and any basic authentication, if required.Usage
Local environment setup
In order to test the SDK with a Proxy, I've followed these steps to set up a local proxy:
Create a class or test to try the code snippet in the section above. Requests made by classes, like tests, should now come through.
References
The feature was originally requested here #91
Testing
Checklist