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.
- 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
