You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stratos.apache.org by Sajith Kariyawasam <sa...@wso2.com> on 2015/01/31 13:22:29 UTC
addCartridge operation checks for already deployed cartridges in two places
Hi Devs,
I noted that in addCartridge operation, existing cartridge check is done
twice.
In StratosApiV41 --> CartridgeBean cartridgeBean =
StratosApiV41Utils.getCartridgeForValidate(cartridgeType);
Also in CloudcontrollerServiceImpl -->
cloudControllerContext.getCartridge(cartridgeType)
I think the check in StratosApi (rest endpoint) is redundant, and that is
not a responsibility of rest endpoint, also this will be an overhead to do
a service call.
Any reason for adding it there..?
Thanks,
Sajith
Re: addCartridge operation checks for already deployed cartridges in
two places
Posted by Imesh Gunaratne <im...@apache.org>.
Yes that's a good suggestion. If we could introduce a generic entity not
found exception and catch it in API exception handler we could do it.
However this might not be straight forward with Axis2 service stubs, we
could give it a try.
Thanks
On Sat, Jan 31, 2015 at 10:23 PM, Sajith Kariyawasam <sa...@wso2.com>
wrote:
> Well, shouldn't it be like we catch the custom exception in StratosApi (in
> this case CartridgeAlreadyExistsException), and return the relevant
> response status code to the end user. Otherwise, we are not make use of
> those custom exceptions
>
> On Sat, Jan 31, 2015 at 9:39 PM, Imesh Gunaratne <im...@apache.org> wrote:
>
>> IMO we need validations in both places. The reason being that in REST API
>> if we do not check the availability of a resource before executing an
>> operation it may raise a HTTP 500 to the end user as a result of the
>> exception raised by the backend service.
>>
>> Thanks
>>
>> On Sat, Jan 31, 2015 at 5:52 PM, Sajith Kariyawasam <sa...@wso2.com>
>> wrote:
>>
>>> Hi Devs,
>>>
>>> I noted that in addCartridge operation, existing cartridge check is done
>>> twice.
>>>
>>> In StratosApiV41 --> CartridgeBean cartridgeBean =
>>> StratosApiV41Utils.getCartridgeForValidate(cartridgeType);
>>>
>>> Also in CloudcontrollerServiceImpl -->
>>> cloudControllerContext.getCartridge(cartridgeType)
>>>
>>> I think the check in StratosApi (rest endpoint) is redundant, and that
>>> is not a responsibility of rest endpoint, also this will be an overhead to
>>> do a service call.
>>>
>>> Any reason for adding it there..?
>>>
>>> Thanks,
>>> Sajith
>>>
>>
>>
>>
>> --
>> Imesh Gunaratne
>>
>> Technical Lead, WSO2
>> Committer & PMC Member, Apache Stratos
>>
>
>
--
Imesh Gunaratne
Technical Lead, WSO2
Committer & PMC Member, Apache Stratos
Re: addCartridge operation checks for already deployed cartridges in
two places
Posted by Sajith Kariyawasam <sa...@wso2.com>.
Well, shouldn't it be like we catch the custom exception in StratosApi (in
this case CartridgeAlreadyExistsException), and return the relevant
response status code to the end user. Otherwise, we are not make use of
those custom exceptions
On Sat, Jan 31, 2015 at 9:39 PM, Imesh Gunaratne <im...@apache.org> wrote:
> IMO we need validations in both places. The reason being that in REST API
> if we do not check the availability of a resource before executing an
> operation it may raise a HTTP 500 to the end user as a result of the
> exception raised by the backend service.
>
> Thanks
>
> On Sat, Jan 31, 2015 at 5:52 PM, Sajith Kariyawasam <sa...@wso2.com>
> wrote:
>
>> Hi Devs,
>>
>> I noted that in addCartridge operation, existing cartridge check is done
>> twice.
>>
>> In StratosApiV41 --> CartridgeBean cartridgeBean =
>> StratosApiV41Utils.getCartridgeForValidate(cartridgeType);
>>
>> Also in CloudcontrollerServiceImpl -->
>> cloudControllerContext.getCartridge(cartridgeType)
>>
>> I think the check in StratosApi (rest endpoint) is redundant, and that is
>> not a responsibility of rest endpoint, also this will be an overhead to do
>> a service call.
>>
>> Any reason for adding it there..?
>>
>> Thanks,
>> Sajith
>>
>
>
>
> --
> Imesh Gunaratne
>
> Technical Lead, WSO2
> Committer & PMC Member, Apache Stratos
>
Re: addCartridge operation checks for already deployed cartridges in
two places
Posted by Imesh Gunaratne <im...@apache.org>.
IMO we need validations in both places. The reason being that in REST API
if we do not check the availability of a resource before executing an
operation it may raise a HTTP 500 to the end user as a result of the
exception raised by the backend service.
Thanks
On Sat, Jan 31, 2015 at 5:52 PM, Sajith Kariyawasam <sa...@wso2.com> wrote:
> Hi Devs,
>
> I noted that in addCartridge operation, existing cartridge check is done
> twice.
>
> In StratosApiV41 --> CartridgeBean cartridgeBean =
> StratosApiV41Utils.getCartridgeForValidate(cartridgeType);
>
> Also in CloudcontrollerServiceImpl -->
> cloudControllerContext.getCartridge(cartridgeType)
>
> I think the check in StratosApi (rest endpoint) is redundant, and that is
> not a responsibility of rest endpoint, also this will be an overhead to do
> a service call.
>
> Any reason for adding it there..?
>
> Thanks,
> Sajith
>
--
Imesh Gunaratne
Technical Lead, WSO2
Committer & PMC Member, Apache Stratos