Re: [PATCH] adaptation_service ACL

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.

Amos
Received on Mon Nov 11 2013 - 07:11:23 MST