camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Claus Ibsen <claus.ib...@gmail.com>
Subject Re: Consul ServiceCall EIP
Date Wed, 01 Jun 2016 13:34:46 GMT
Ah yeah it should, you are welcome to fix that.

On Wed, Jun 1, 2016 at 3:11 PM, Luca Burgazzoli <lburgazzoli@gmail.com> wrote:
> 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.ibsen@gmail.com> wrote:
>> On Wed, Jun 1, 2016 at 2:53 PM, Luca Burgazzoli <lburgazzoli@gmail.com> wrote:
>>> ---
>>> Luca Burgazzoli
>>>
>>>
>>> On Wed, Jun 1, 2016 at 2:44 PM, Luca Burgazzoli <lburgazzoli@gmail.com>
wrote:
>>>> ---
>>>> Luca Burgazzoli
>>>>
>>>>
>>>> On Wed, Jun 1, 2016 at 9:13 AM, Luca Burgazzoli <lburgazzoli@gmail.com>
wrote:
>>>>> Will fix them today
>>>>> ---
>>>>> Luca Burgazzoli
>>>>>
>>>>>
>>>>> On Wed, Jun 1, 2016 at 8:56 AM, Claus Ibsen <claus.ibsen@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 <lburgazzoli@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.ibsen@gmail.com>
wrote:
>>>>>>>> On Mon, May 30, 2016 at 10:49 AM, Luca Burgazzoli <lburgazzoli@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.ibsen@gmail.com>
wrote:
>>>>>>>>>> On Mon, May 30, 2016 at 10:16 AM, Luca Burgazzoli
<lburgazzoli@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.ibsen@gmail.com> wrote:
>>>>>>>>>>>> On Thu, May 26, 2016 at 7:30 PM, Luca Burgazzoli
<lburgazzoli@gmail.com> wrote:
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> Luca Burgazzoli
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, May 26, 2016 at 7:06 PM, Claus
Ibsen <claus.ibsen@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 <lburgazzoli@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



-- 
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2

Mime
View raw message