I did had a look but as it does not use
KubernetesDnsServiceCallExpression I'm having hard times to figure out
how it works, shouldn't KubernetesDnsServiceCallExpression be used
instead of KubernetesServiceCallExpression ?
---
Luca Burgazzoli
On Wed, Jun 1, 2016 at 2:54 PM, Claus Ibsen <claus.ib...@gmail.com> wrote:
> On Wed, Jun 1, 2016 at 2:53 PM, Luca Burgazzoli <lburgazz...@gmail.com> wrote:
>> ---
>> Luca Burgazzoli
>>
>>
>> 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 ?
>>>
>>
>> Sorry found it, in fact now Env returns an immutable list which
>> contains a single server.
>> BTW, was DNS support complete in Kubernetes ServiceCall ? looking at
>> the code looks like ip/port are never set
>>
>
> The DNS uses a domain name that includes the service name but without
> ip and port number. And then it has some convention for a prefix /
> suffix etc.
>
> But yeah it may need a bit more look and testing running on a k8s platform.
>
>
>
>>>>>
>>>>> - 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
