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
