Skip to content
This repository has been archived by the owner on Mar 7, 2020. It is now read-only.

Add Logger Support #11

Open
kdavisk6 opened this issue Aug 16, 2018 · 4 comments
Open

Add Logger Support #11

kdavisk6 opened this issue Aug 16, 2018 · 4 comments
Labels

Comments

@kdavisk6
Copy link
Member

The current implementation is locked to SLF4J. We replace this with support for the Logger abstraction.

@kptfh
Copy link
Collaborator

kptfh commented Aug 17, 2018

Do you mean feign.Logger?

@kdavisk6
Copy link
Member Author

Yes.

@kptfh
Copy link
Collaborator

kptfh commented Aug 17, 2018

'feign.Logger' is not designed for reactive work. Can introduce ReactorLogger but don't have enough reason for this as SLF4J is also a high level abstraction.

@kdavisk6
Copy link
Member Author

I disagree, and we shouldn't ask users of OpenFeign to throw away concepts that are prevalent across all of our other integrations. There are ways to take a synchronous process and make it reactive. Take the reactor documentation itself, B.1. How do I wrap a synchronous, blocking call?.

Feign favor Composition over Inheritance. It is reasonable for us to be able add support for both a native Reactive approach and an approach that allows users to use existing Synchronous Feign constructs. By doing so we can increase the likely hood of this projects adoption.

@velo velo added the API label Aug 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants