-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Modify HttpRequestTemplate and HttpResourceGroup to be DI friendly #138
Conversation
ribbon-pull-requests #137 FAILURE |
@elandau Thanks for the DI work. I am wondering if the HttpRequestTemplateFactory is necessary. It seems to me that being able to inject HttpResourceGroupFactory will satisfy our use case since it can further control the behavior of creating HttpRequestTemplate. |
…esourceGroupFactory
ribbon-pull-requests #141 FAILURE |
ribbon-pull-requests #142 FAILURE |
ribbon-pull-requests #143 SUCCESS |
ribbon-pull-requests #144 SUCCESS |
@@ -125,7 +125,7 @@ public boolean runExample() { | |||
} | |||
} | |||
); | |||
return resultObservable.materialize().toBlocking().last(); | |||
return resultObservable.materialize().toBlockingObservable().last(); |
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.
Can you revert this change? toBlockingObservable() is deprecated in 0.19.
…RequestTemplateFactory. Move ClientConfigFactory to core. Revert the changes to toBlockingObservable().
Some refactoring and renaming
ribbon-pull-requests #145 SUCCESS |
ribbon-pull-requests #146 SUCCESS |
Modify RxMovieClientTestBase to use ephemeral ports instead of Random ports
ribbon-pull-requests #147 FAILURE |
ribbon-pull-requests #149 FAILURE |
Will send a PR to fix the test not to rely on external host for redirecting. |
Use local mocked HTTP server to test redirecting.
ribbon-pull-requests #150 SUCCESS |
} | ||
|
||
@Override | ||
public <T> T from(Class<T> classType, HttpResourceGroup resourceGroup) { |
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.
Do we need this method? It does not use any object attributes, and is equivalent to calling Ribbon.from directly.
…for future extension. Removed APIs to create proxy from ResourceGroup.
Make RibbonResourceFactory and RibbonTransportFactory abstract class for future extension. Removed APIs to create proxy from ResourceGroup.
ribbon-pull-requests #151 FAILURE |
Added test for custom binding of ClientConfigFactory.
ribbon-pull-requests #152 SUCCESS |
Modify HttpRequestTemplate and HttpResourceGroup to be DI friendly
To enable ribbon in Guice just add RibbonModule() when creating the injector (either via Guice or Governator). Alternatively one can override the bindings for HttpResourceGroupFactory and HttpRequestTemplateFactory to override HttpResourceGroup and HttpRequestTemplate.