E53A Make the InvocationHandler plugable via a new InvocationHandlerFactory by spencergibb · Pull Request #105 · OpenFeign/feign · GitHub
[go: up one dir, main page]
Skip to content

Make the InvocationHandler plugable via a new InvocationHandlerFactory#105

Closed
spencergibb wants to merge 1 commit intoOpenFeign:masterfrom
spencergibb:plugable-invocation-handler
Closed

Make the InvocationHandler plugable via a new InvocationHandlerFactory#105
spencergibb wants to merge 1 commit intoOpenFeign:masterfrom
spencergibb:plugable-invocation-handler

Conversation

@spencergibb
Copy link
Contributor
@spencergibb spencergibb commented Apr 13, 2014

No description provided.

@davidmc24
Copy link
Contributor
davidmc24 commented Apr 13, 2014

Looks like a reasonable change to me. What sort of alternate behaviors do you have need of for the InvocationHandler?

@cloudbees-pull-request-builder
Copy link
cloudbees-pull-request-builder commented Apr 13, 2014

feign-pull-requests #143 FAILURE
Looks like there's a problem with this pull request

@spencergibb
Copy link
Contributor Author
spencergibb commented Apr 13, 2014

I'm trying to integrate some of the Netflix OSS components and spring halfpipe/next

I really like feign, and like Hystrix, but want them to work together:

@RequestLine("GET /v1/posts")
HystrixExecutable<List<Post>> postsExecuatble();

I want to auto-configure the HystrixCommand using spring. I had a Proxy of the Feign Proxy, but I had to jump through hoops and it added an extra layer.

Not sure about the LoggerTest failure. I get different errors on my box than cloudbees.

codefromthecrypt
codefromthecrypt reviewed May 25, 2014
View reviewed changes
core/src/main/java/feign/FeignInvocationHandler.java
Copy link
Contributor
@codefromthecrypt codefromthecrypt May 25, 2014

Choose a reason for hiding this comment

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

nit: axe the minutea :)

@codefromthecrypt
Copy link
Contributor
codefromthecrypt commented May 25, 2014

@spencergibb So, I've noticed some things need to be changed to support what you've sketched out more nicely. If you don't mind, I'll give a stab at doing so, because I think your idea is a good one.

codefromthecrypt pushed a commit that referenced this pull request May 25, 2014
Fix #105: expose hook for reflective dispatch.
daefbf1
codefromthecrypt pushed a commit that referenced this pull request May 25, 2014
Fix #105: expose hook for reflective dispatch.
44583db
codefromthecrypt pushed a commit that referenced this pull request May 25, 2014
Fix #105: expose hook for reflective dispatch.
95a0f6b
@codefromthecrypt codefromthecrypt mentioned this pull request May 25, 2014
Merged
@codefromthecrypt
Copy link
Contributor
codefromthecrypt commented May 25, 2014

Let's continue discussing here! #109

Lemme know if this works for you.

velo pushed a commit that referenced this pull request Oct 8, 2024
Fix #105: expose hook for reflective dispatch.
e52f2ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@spencergibb @davidmc24 @cloudbees-pull-request-builder @codefromthecrypt
0