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?
A simple example is when you want log requests based on icap service.
For example use icap_log to log icap requests to a service.
>
> 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.
>
OK for all of them.
>
> These do not need another round of review after fixing. +1 on commit
> with the above changes.
OK. I will wait today for more comments and I commit if there are no new
requests.
>
> Amos
>
>
Received on Mon Nov 11 2013 - 12:11:32 MST
This archive was generated by hypermail 2.2.0 : Tue Nov 12 2013 - 12:00:12 MST