add HystrixCommand support#280
Conversation
|
NetflixOSS » feign » feign-pull-requests #160 SUCCESS |
There was a problem hiding this comment.
Required to change the return type to the actual type to send on to the decoder. If the return type is HystrixCommand<User>, User is set as the return type.
There was a problem hiding this comment.
Nope, good catch.
|
wondering out loud.. Seems the problem is trying to work within the invocation handler. What if you subclass ReflectiveFeign's constructor has everything you need (I think) to ensure the right type gets into MethodMetadata in the first place. Not that this would work, but.. new HystrixedFeignBuilder()...not setting invocation factory.. build() |
|
So move the mangling the methodMetadata to the new builder and automatically set the invocation handler there? |
|
or maybe add a method to InvocationHandlerFactory, exposing the setters public as you mentioned. |
|
Either way, I suppose.. with Hystrix you don't need a new builder per-se.. gets more complicated with things like RxNetty which need to set multiple builder methods to work. Just thinking if we should continue with invocationhandlerfactory as a builder method, or basically say, make a new builder if you need to set a invocationhandlerfactory. wdyt? |
|
Not sure. I could knock both out and see what they look like. Then we could compare? |
|
yeah.. somewhere in the scope of build wire in the ability to decorate
methodmetadata somewhere between contract.parseAndValidatateMetadata
and where it is actually used. Ex. if you override the builder, you
could make a filtering contract, which manipulates the methodmetadata.
ex.
```
public Builder contract(Contract contract) {
this.contract = new FirstTypeIsReturnValContract(contract);
return this;
}
FirstTypeIsReturnValContract implements Contract {
override parseAndValidatateMetadata
filter return and toss the enclosing parameterized type
```
|
There was a problem hiding this comment.
shouldn't it logged not printed?
There was a problem hiding this comment.
spencergibb
force-pushed
the
hystrix
branch
2 times, most recently
from
2eaa6bd to
ab970d4
Compare
There was a problem hiding this comment.
or to apply hystrix to existing feign methods.
There was a problem hiding this comment.
something like..
// plain methods will inherit hystrix synchronous behavior
There was a problem hiding this comment.
s/creates a builder that //
builder isn't the headline
|
NetflixOSS » feign » feign-pull-requests #166 SUCCESS |
|
|
|
NetflixOSS » feign » feign-pull-requests #167 SUCCESS |
add HystrixCommand support
4 participants
fixes gh-189