> On Feb. 28, 2014, 6:51 a.m., Matt Jordan wrote:
> > I'm not sure I understand the need for this patch.
> >
> > Setting a configuration option twice - when that option doesn't support
> > being set multiple times - would generally have undefined behaviour. Your
> > patch changes it so that Asterisk reads the last defined value, as opposed
> > to the first. How is that better?
>
> Leif Madsen wrote:
> It's better when you want to deploy Asterisk in a DevOps environment.
> What happens is you define the "default" behavior in the main file that gets
> deployed. From there you then override that behavior in an included file
> which is defined and built via DevOps (usually through a template of some
> sort that contains information for the particular machine you're deploying).
>
> The example is not a good one, because obviously you would never deploy
> the file in the example provided.
>
> ; logger.conf
> [general]
> queue_log=no
>
> #include logger.conf.d/logger.conf.local
>
>
> ; logger.conf.d/logger.conf.local
> [general]
> queue_log=yes ; we've deployed a queue server, so enable queue logging
>
>
> Primary example of it in use available at
> https://github.com/kickstandproject/kickstandproject-asterisk/tree/master/templates/etc/asterisk
>
> wdoekes wrote:
> +1 on iterating over the configs options. Most modules do, some don't.
> Consistency is nice.
>
> Please do add a note to the upgrade file though. Perhaps someone has
> already worked around this by placing the #include logger.conf.d statement at
> the top of the file.
I can see how this might be better for that deployment scenario.
I compared this approach against the Config Framework (config_options.c). It
uses an ast_variable_browse as well (which is not surprising, since it has to
parse through a list of key/value pairs), so this does take this into line with
the recommended (tm) way of doing things:
int aco_process_category_options(struct aco_type *type, struct ast_config *cfg,
const char *cat, void *obj)
{
struct ast_variable *var;
for (var = ast_variable_browse(cfg, cat); var; var = var->next) {
if (aco_process_var(type, cat, var, obj)) {
return -1;
}
}
return 0;
}
That being said, why not just bite the bullet and use the configuration
framework for logger.conf?
- Matt
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3279/#review10991
-----------------------------------------------------------
On Feb. 27, 2014, 7:44 p.m., Paul Belanger wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3279/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2014, 7:44 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> This patch allows you to override the [general] section of logger.conf,
> making it the same functionality as the [logfiles] sections.
>
>
> Diffs
> -----
>
> trunk/main/logger.c 409111
>
> Diff: https://reviewboard.asterisk.org/r/3279/diff/
>
>
> Testing
> -------
>
> local development. Setup
>
> [general]
> queue_log = no
> queue_log = yes
>
> Queue logfiles were created.
>
>
> Thanks,
>
> Paul Belanger
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
