---
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

Reply via email to