On Wed, Jun 1, 2016 at 2:44 PM, Luca Burgazzoli <lburgazz...@gmail.com> wrote:
> ---
> 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 ?
>
Looks like in DefaultServiceCallProcessor
And in current code you can see in camel-kubernetes how
KubernetesDnsServiceCallProcessor and
KubernetesEnvironmentServiceCallProcessor do not use the load balancer
/ server list strategy etc.
>>>
>>> - 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
--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2
