Re: [PATCH] adaptation_service ACL

From: Tsantilas Christos <chtsanti_at_users.sourceforge.net>
Date: Tue, 12 Nov 2013 16:50:03 +0200

On 11/11/2013 09:11 AM, Amos Jeffries wrote:
> On 7/11/2013 11:27 p.m., Tsantilas Christos wrote:
>> This patch adds the new ACL adaptation_service, to match the name of:
>>
>> - an adaptation service or group that had been applied to the master
>> transaction in the past
>> - an adaptation service or group that is being applied to the master
>> transaction now
>>
>> An adaptation group is formed by adaptation_service_chain or
>> adaptation_service_set directives. Both REQMOD and RESPMOD services,
>> successful or failed service applications matches this acl.
>>
>> This is a Measurement Factory project
>>
>
> 1) Can we please get a bit of insight into why/how this is useful addition?
>
> The code looks mostly fine, but the text here for proposed commit
> message leaves me wondering why you spent any time coding this up and
> who will ever find it useful.
>
>
> 2) code audit ...
>
> in src/acl/AdaptationService.cc:
>
> * please use HttpRequest::Pointer instead of HttpRequest* for the local
> variable.
>
> in src/acl/AdaptationServiceData.cc:
>
> * please use DBG_CRITICAL instead of '0' for debug level. Also, please
> prefix with "FATAL:" the debug which is followed by self_destruct() to
> highlight the issue.
>
>
> These do not need another round of review after fixing. +1 on commit
> with the above changes.

patch applied to trunk with the above changes.

>
> Amos
>
>
Received on Tue Nov 12 2013 - 14:50:34 MST

This archive was generated by hypermail 2.2.0 : Tue Nov 12 2013 - 12:00:12 MST