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
