spring-attic / http

Apache License 2.0
7 stars 12 forks source link

@EnableDiscovery for HttpSource #11

Open novettaberin opened 7 years ago

novettaberin commented 7 years ago

The usefulness of the HttpSource starter app is severely limited by the fact it does not support finding it through the discovery service.

Proposal:

This allows one service to kick off processing with minimal setup by setting up it's rest client to find the HttpSource by a simple URL like "http://http-source/" and we don't have to worry about assuming what port it is listening on or what docker image it is being hosted in.

artembilan commented 7 years ago

Thank you for the pointer @bericoberin !

The default service name, instance id and port, taken from the Environment, are ${spring.application.name}, the Spring Context ID and ${server.port} respectively.

Meanwhile the binder-specific target Boot application has this application.properties:

spring.application.name=${vcap.application.name:http-source}
info.app.name=@project.artifactId@
info.app.description=@project.description@
info.app.version=@project.version@

Therefore we are good to go just with the @EnableDiscoveryClient. Although we might need to think how to add all possible Discovery Clients implementations and activate only that one which has been selected by end-user:

http://cloud.spring.io/spring-cloud-static/Dalston.SR1/#__enablediscoveryclient

How to activate

Including a dependency on org.springframework.cloud:spring-cloud-starter-zookeeper-discovery will enable auto-configuration that will setup Spring Cloud Zookeeper Discovery.

At the same time we might need to think about making this feature for all the out-of-the-box applications, since they are Spring Boot apps as well and there is definitely fully supported Boot management and health check.

WDYT, @viniciusccarvalho , @sobychacko ?

viniciusccarvalho commented 7 years ago

@artembilan When I suggested this issue, I forgot about the many implementations of the discovery client. This can be a bit of a problem.

The good news is that is very, very straightforward adding it yourself @bericoberin, so I don't think it's a major blocker in adoption at the moment.

There's some discussion internally on replacing the http source by a full blown gateway, with many goodies such as rate limiting, service registration, routing and many others. I need to pickup that discussion with Spencer before we release 2.0

novettaberin commented 7 years ago

OK, but keep in mind in my situation I already have a Discovery server, Gateway (using discovery), Config Server, Zipkin integration etc. If the answer is that I need to fork it and do what I need I understand. I just need something that can live behind the gateway and be discoverable by the other components that need to invoke it. Maybe I might need OAuth support, but that's about the only other thing.

On Thu, Jun 29, 2017 at 4:05 PM, Vinicius Carvalho <notifications@github.com

wrote:

@artembilan https://github.com/artembilan When I suggested this issue, I forgot about the many implementations of the discovery client. This can be a bit of a problem.

The good news is that is very, very straightforward adding it yourself @bericoberin https://github.com/bericoberin, so I don't think it's a major blocker in adoption at the moment.

There's some discussion internally on replacing the http source by a full blown gateway, with many goodies such as rate limiting, service registration, routing and many others. I need to pickup that discussion with Spencer before we release 2.0

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-cloud-stream-app-starters/http/issues/11#issuecomment-312088791, or mute the thread https://github.com/notifications/unsubscribe-auth/Ab7f3gc2JwUFYYsx2DEyOfKwrWoMvSpkks5sJANtgaJpZM4OJOHc .

artembilan commented 7 years ago

@bericoberin ,

Thank you for your feedback. Well, if you are good with patching existing app, that's great for your time now. We definitely will look into the feature as out-of-the-box leverage.

@viniciusccarvalho ,

thank you for you point of view!

Would you mind anyway to explain more how HTTP Source (essentially @Controller) can be replace with some gateway? How those " rate limiting, service registration, routing etc." are related to handling end-user requests and emitting the result to the SCSt flow? Thanks

novettaberin commented 7 years ago

We are using Zuul Gateway which integrates with discovery and helps with load balancing, etc. It's got a lot of features we need. In our case, we don't need rate limiting because we have other ways of dealing with back pressure, and we need every event to put information in the queue.

On Jun 30, 2017 10:59 AM, "Artem Bilan" notifications@github.com wrote:

@bericoberin https://github.com/bericoberin ,

Thank you for your feedback. Well, if you are good with patching existing app, that's great for your time now. We definitely will look into the feature as out-of-the-box leverage.

@viniciusccarvalho https://github.com/viniciusccarvalho ,

thank you for you point of view!

Would you mind anyway to explain more how HTTP Source (essentially @Controller) can be replace with some gateway? How those " rate limiting, service registration, routing etc." are related to handling end-user requests and emitting the result to the SCSt flow? Thanks

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-cloud-stream-app-starters/http/issues/11#issuecomment-312290727, or mute the thread https://github.com/notifications/unsubscribe-auth/Ab7f3ncz0CbzQ0WsaqzbBXB9BTFOCSrSks5sJQ0_gaJpZM4OJOHc .