--- Luca Burgazzoli
On Wed, Jun 1, 2016 at 9:13 AM, Luca Burgazzoli <lburgazz...@gmail.com> wrote: > Will fix them today > --- > Luca Burgazzoli > > > On Wed, Jun 1, 2016 at 8:56 AM, Claus Ibsen <claus.ib...@gmail.com> wrote: >> Hi >> >> Looks good there is only a few spots I have some comments. >> >> - There is some code that has not null check for load balancer / >> server list strategy. The kubernetes component can do without that >> when you use ENV or DNS as lookup. Can you point me out to such places ? >> >> - There was a place where you do some logging and it says Consul when >> its in fact generic >> >> - You have label("ServiceCall") but that label name seems wrong. eg >> notice that label is like a "tag" not a leading label in a UI form >> etc. Yeah we should maybe have named it "tag" or "group" instead. >> >> - In the Java DSL we often have it return Type so you do not change >> the type and makes the fluent easier to use. Only if you need to do >> some more advanced configuration then we have other methods that >> return the ServiceCallDefinition type so you can call specialized >> method on that in the Java DSL. >> >> >> >> serviceCall().name("foo").loadBalancer("random").end() >> // here you have the ServiceCallDefinition in the DSL to call >> specialized methods. But have to call end() to end it >> >> And then with short hand that just return type >> >> serviceCall("foo") >> // here the Type is not changed and you just want to call the >> service foo, and NOT do any other configuration. Then you do not need >> the end() >> >> >> >> >> >> On Tue, May 31, 2016 at 4:54 PM, Luca Burgazzoli <lburgazz...@gmail.com> >> wrote: >>> I've committed some more code to my branch [2], it is not yet complete >>> (i.e. I still have to make ribbon code to use the new common classes >>> and some APIs are not clean enough) but a feedback would be really >>> welcome. >>> >>> --- >>> Luca Burgazzoli >>> >>> >>> On Mon, May 30, 2016 at 10:55 AM, Claus Ibsen <claus.ib...@gmail.com> wrote: >>>> On Mon, May 30, 2016 at 10:49 AM, Luca Burgazzoli <lburgazz...@gmail.com> >>>> wrote: >>>>> So like serviceCall("myServiceCall").configurationRef("myConf") ? >>>> >>>> Yeah there is already a DSL for that named >>>> serviceCallConfiguration("myConf") >>>> >>>> >>>> >>>> >>>>> --- >>>>> Luca Burgazzoli >>>>> >>>>> >>>>> On Mon, May 30, 2016 at 10:26 AM, Claus Ibsen <claus.ib...@gmail.com> >>>>> wrote: >>>>>> On Mon, May 30, 2016 at 10:16 AM, Luca Burgazzoli >>>>>> <lburgazz...@gmail.com> wrote: >>>>>>> do you mean something like serviceCallRef("myServiceCall") ? >>>>>>> --- >>>>>> >>>>>> No you need to provide >>>>>> >>>>>> - a) name of service to call >>>>>> - b) reference to configuration of service >>>>>> >>>>>> a = mandatory >>>>>> b = optional. As if there is only 1 configuration then use that. >>>>>> >>>>>> >>>>>> >>>>>>> Luca Burgazzoli >>>>>>> >>>>>>> >>>>>>> On Sun, May 29, 2016 at 9:42 AM, Claus Ibsen <claus.ib...@gmail.com> >>>>>>> wrote: >>>>>>>> On Thu, May 26, 2016 at 7:30 PM, Luca Burgazzoli >>>>>>>> <lburgazz...@gmail.com> wrote: >>>>>>>>> --- >>>>>>>>> Luca Burgazzoli >>>>>>>>> >>>>>>>>> >>>>>>>>> On Thu, May 26, 2016 at 7:06 PM, Claus Ibsen <claus.ib...@gmail.com> >>>>>>>>> wrote: >>>>>>>>>> Hi Luca >>>>>>>>>> >>>>>>>>>> Yeah its good to get more eyes on this new set of code. When I >>>>>>>>>> created >>>>>>>>>> kubernetes and ribbon there was sure some overlap of code that could >>>>>>>>>> be shared, but I didn't push for much default/abstract code in >>>>>>>>>> camel-core because there its new code and I also wanted to see what >>>>>>>>>> consul, etcd, zookeeper and other distributed systems may >>>>>>>>>> need/require. >>>>>>>>>> >>>>>>>>>> I like the idea of the impl.remote package to have some base >>>>>>>>>> implementation there. >>>>>>>>>> >>>>>>>>>> Your current branch [2] has a good set of reusable code although its >>>>>>>>>> tied to consul currently, so that would need to be made abstract so >>>>>>>>>> it >>>>>>>>>> can be reuse by kuernetes and maybe also ribbon as well (where it >>>>>>>>>> makes sense). >>>>>>>>> >>>>>>>>> I've removed some consul specific stuffs that I left by mistake, >>>>>>>>> should be >>>>>>>>> a little tidy now. >>>>>>>>> >>>>>>>>> An aspect to take into account is how to make it easy to configure >>>>>>>>> ServiceCallServerListStrategy in case we use >>>>>>>>> DefaultServiceCallProcessor >>>>>>>>> maybe something like: >>>>>>>>> >>>>>>>>> serviceCall() >>>>>>>>> .name("my-service") >>>>>>>>> .roundRobinLoadBalancer() >>>>>>>>> .consulServerListStrategy() >>>>>>>>> .type(Strategy.ON_DEMAND) >>>>>>>>> .url("http://consul-host:8500") >>>>>>>>> .dc("west") >>>>>>>>> .end() >>>>>>>>> >>>>>>>>> Too ugly ? >>>>>>>>> >>>>>>>> >>>>>>>> Yeah possible - its always tricky to find the right balance. >>>>>>>> >>>>>>>> I wonder if you may want to do this in the configuration, and then in >>>>>>>> the routes with serviceCall you then just need to refer to the service >>>>>>>> name / url to be used - then all the round robin, service list, and so >>>>>>>> on are configured outside the route in the configuration. >>>>>>>> >>>>>>>> We could also leave those in the route DSLs as well so you can >>>>>>>> override the configuration, so you can use >>>>>>>> >>>>>>>> serviceCall().name("foo").consulConfiguraiton().dc("west").end() >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> But then on the other hand if you just want to call a single service >>>>>>>> you may want to do it all in the route without the configuration. But >>>>>>>> if we look at rest-dsl then it separates the configuration from the >>>>>>>> REST endpoint. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>>> >>>>>>>>>> An aspect we haven't added yet could be to find out if we can expose >>>>>>>>>> some JMX attributes and operations for thise service call EIP as >>>>>>>>>> well? >>>>>>>>>> And then maybe some Camel commands so you can manage/list it from >>>>>>>>>> karaf and other CLIs. But this part is more "nice to have" and a bit >>>>>>>>>> "eye candy" but still somewhat cool. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Thu, May 26, 2016 at 4:13 PM, Luca Burgazzoli >>>>>>>>>> <lburgazz...@gmail.com> wrote: >>>>>>>>>>> Hello, >>>>>>>>>>> >>>>>>>>>>> I'm playing a little bit with the new ServiceCall EIP by adding >>>>>>>>>>> support for >>>>>>>>>>> consul service discovery and I've committed some code in my own >>>>>>>>>>> branch [1]. >>>>>>>>>>> >>>>>>>>>>> I borrowed most of the code from camel-kubernetes and as it ended >>>>>>>>>>> up being >>>>>>>>>>> almost a clone, I've tried to make some base/default classes as >>>>>>>>>>> what really >>>>>>>>>>> make the difference is the implementation of >>>>>>>>>>> ServiceCallServerListStrategy >>>>>>>>>>> and ServiceCallLoadBalancer so to add a simple discovery engine you >>>>>>>>>>> only >>>>>>>>>>> need to implement your own ServiceCallServerListStrategy and >>>>>>>>>>> eventually your >>>>>>>>>>> own ServiceCallLoadBalancer (i.e. for ribbon). >>>>>>>>>>> >>>>>>>>>>> Does it make sense ? >>>>>>>>>>> >>>>>>>>>>> [1] https://github.com/lburgazzoli/apache-camel/tree/CAMEL-9989 >>>>>>>>>>> [2] >>>>>>>>>>> https://github.com/apache/camel/compare/master...lburgazzoli:CAMEL-9989?expand=1 >>>>>>>>>>> >>>>>>>>>>> --- >>>>>>>>>>> Luca Burgazzoli >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Claus Ibsen >>>>>>>>>> ----------------- >>>>>>>>>> http://davsclaus.com @davsclaus >>>>>>>>>> Camel in Action 2: https://www.manning.com/ibsen2 >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Claus Ibsen >>>>>>>> ----------------- >>>>>>>> http://davsclaus.com @davsclaus >>>>>>>> Camel in Action 2: https://www.manning.com/ibsen2 >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Claus Ibsen >>>>>> ----------------- >>>>>> http://davsclaus.com @davsclaus >>>>>> Camel in Action 2: https://www.manning.com/ibsen2 >>>> >>>> >>>> >>>> -- >>>> Claus Ibsen >>>> ----------------- >>>> http://davsclaus.com @davsclaus >>>> Camel in Action 2: https://www.manning.com/ibsen2 >> >> >> >> -- >> Claus Ibsen >> ----------------- >> http://davsclaus.com @davsclaus >> Camel in Action 2: https://www.manning.com/ibsen2