On Tue, Jun 7, 2016 at 11:53 AM, Luca Burgazzoli <lburgazz...@gmail.com> wrote:
> I've pushed some new code and polished the old one to my branch so now
> we have service calls for:
>
> - camel-kubernetes
> - camel-consul
> - camel-etcd
> - camel-dns
> - camel-ribbon
>
> Ribbon work a little bit differently from the others so it does make
> use of only a small subset of common base classes.
> Base classes may need a little bit more love and clean-up but I would
> like to merge them if you agree.
>
Awesome work Luca. Thanks for taking a 2nd set of eyes on this.
Welcome to merge the code.
> A questions: would it make sense to add more stuffs like attributes,
> tags and priority to ServiceCallServer ?
>
> Rationale is:
> - add more options to the final processor through attributes in
> addition to ip and port
> - capability to have a generic filter to be used in server list filtering
> - create a "load balancer" that can choose the server based on the priority
>
Yeah you may want to LB among servers that are in the same zone as
you, or closer to you etc.
Maybe you can log a JIRA with some of these ideas.
And sure after the merge, you are welcome to look at this as well.
>
>
> ---
> Luca Burgazzoli
>
>
> On Wed, Jun 1, 2016 at 3:34 PM, Claus Ibsen <claus.ib...@gmail.com> wrote:
>> Ah yeah it should, you are welcome to fix that.
>>
>> On Wed, Jun 1, 2016 at 3:11 PM, Luca Burgazzoli <lburgazz...@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.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
>>
>>
>>
>> --
>> 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
