[asterisk-bugs] [JIRA] (ASTERISK-28951) Inconsistent behaviour queues.conf when there is (not) a [general] section

Friendly Automation (JIRA) noreply at issues.asterisk.org
Thu Jul 9 05:23:26 CDT 2020


    [ https://issues.asterisk.org/jira/browse/ASTERISK-28951?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=251384#comment-251384 ] 

Friendly Automation commented on ASTERISK-28951:
------------------------------------------------

Change 14596 merged by Joshua Colp:
app_queue: (Breaking change) shared_lastcall and autofill default to no

[https://gerrit.asterisk.org/c/asterisk/+/14596|https://gerrit.asterisk.org/c/asterisk/+/14596]

> Inconsistent behaviour queues.conf when there is (not) a [general] section
> --------------------------------------------------------------------------
>
>                 Key: ASTERISK-28951
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-28951
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Applications/app_queue
>    Affects Versions: 13.34.0
>            Reporter: Walter Doekes
>            Assignee: Walter Doekes
>            Severity: Trivial
>
> app_queue suggests these "defaults" at the top of the file:
> {code}
> /*! \brief queues.conf [general] option */
> static int autofill_default = 1;
> /*! \brief queues.conf [general] option */
> static int montype_default = 0;
> /*! \brief queues.conf [general] option */
> static int shared_lastcall = 1;
> {code}
> The queue reload code contains this:
> {code}
> /*! Set the global queue parameters as defined in the "general" section of queues.conf */
> static void queue_set_global_params(struct ast_config *cfg)
> {
> ...
>         autofill_default = 0;
>         if ((general_val = ast_variable_retrieve(cfg, "general", "autofill"))) {
>                 autofill_default = ast_true(general_val);
> ...
>         shared_lastcall = 0;
>         if ((general_val = ast_variable_retrieve(cfg, "general", "shared_lastcall"))) {
>                 shared_lastcall = ast_true(general_val);
> {code}
> And that code is ran here:
> {code}
> static int reload_queues(int reload, struct ast_flags *mask, const char *queuename)
> ...
>         if (!(cfg = ast_config_load("queues.conf", config_flags))) {
> ...
>         while ((cat = ast_category_browse(cfg, cat)) ) {
>                 if (!strcasecmp(cat, "general") && queue_reload) {
>                         queue_set_global_params(cfg);
>                         continue;
> {code}
> That means:
> - if there is a queues.conf
> - it has a {{\[general]}} section (or _had_ a such a section)
> - the autofill_default default is 0, and the shared_lastcall default is 0
> If there is NO {{\[general]}} section:
> - the autofill_default default is 1, and the shared_lastcall default is 1
> That's confusing, and also, it disagrees with the sample cfg:
> {noformat}
> ;    in most cases, you will want to enable this behavior. If you
> ;    do not specify or comment out this option, it will default to yes.
> ;
> ;autofill = no
> ...
> ; The default value is no.
> ;
> ;shared_lastcall=no
> {noformat}
> That last one was changed here:
> {noformat}
> commit 4b50e3f1ee84ae29da6d9eb3cfd9896a49d2394b
> Author: Mark Michelson <mmichelson at digium.com>
> Date:   Mon Aug 27 17:52:16 2012 +0000
>     Fix incorrectly documented option in queues.conf
>     
>     sharedlastcall defaults to "no" not "yes"
> {noformat}
> But the autofill_default was not. And they still differ depending on the existence of the general section.
> Suggested fix:
> {noformat}
> diff --git a/apps/app_queue.c b/apps/app_queue.c
> index 3a572efc2d..1e786fc39f 100644
> --- a/apps/app_queue.c
> +++ b/apps/app_queue.c
> @@ -1363,34 +1363,34 @@ static char *app_ql = "QueueLog" ;
>  static const char * const pm_family = "Queue/PersistentMembers";
>  
>  /*! \brief queues.conf [general] option */
> -static int queue_persistent_members = 0;
> +static int queue_persistent_members;
>  
> -/*! \brief queues.conf per-queue weight option */
> -static int use_weight = 0;
> +/*! \brief Records that one or more queues use weight */
> +static int use_weight;
>  
>  /*! \brief queues.conf [general] option */
> -static int autofill_default = 1;
> +static int autofill_default;
>  
>  /*! \brief queues.conf [general] option */
> -static int montype_default = 0;
> +static int montype_default;
>  
>  /*! \brief queues.conf [general] option */
> -static int shared_lastcall = 1;
> +static int shared_lastcall;
>  
>  /*! \brief queuesrules.conf [general] option */
> -static int realtime_rules = 0;
> +static int realtime_rules;
>  
>  /*! \brief Subscription to device state change messages */
>  static struct stasis_subscription *device_state_sub;
>  
>  /*! \brief queues.conf [general] option */
> -static int update_cdr = 0;
> +static int update_cdr;
>  
>  /*! \brief queues.conf [general] option */
> -static int negative_penalty_invalid = 0;
> +static int negative_penalty_invalid;
>  
>  /*! \brief queues.conf [general] option */
> -static int log_membername_as_agent = 0;
> +static int log_membername_as_agent;
>  
>  /*! \brief name of the ringinuse field in the realtime database */
>  static char *realtime_ringinuse_field;
> @@ -8722,14 +8722,19 @@ static struct ast_custom_function queuememberpenalty_function = {
>  	.write = queue_function_memberpenalty_write,
>  };
>  
> +/*! Reset the global queue rules parameters even if there is no "general" section of queuerules.conf */
> +static void queue_rules_reset_global_params()
> +{
> +	realtime_rules = 0;
> +}
> +
>  /*! Set the global queue rules parameters as defined in the "general" section of queuerules.conf */
>  static void queue_rules_set_global_params(struct ast_config *cfg)
>  {
> -        const char *general_val = NULL;
> -        realtime_rules = 0;
> -        if ((general_val = ast_variable_retrieve(cfg, "general", "realtime_rules"))) {
> -                realtime_rules = ast_true(general_val);
> -        }
> +	const char *general_val = NULL;
> +	if ((general_val = ast_variable_retrieve(cfg, "general", "realtime_rules"))) {
> +		realtime_rules = ast_true(general_val);
> +	}
>  }
>  
>  /*! \brief Reload the rules defined in queuerules.conf
> @@ -8764,6 +8769,7 @@ static int reload_queue_rules(int reload)
>  			ast_free(pr_iter);
>  		ast_free(rl_iter);
>  	}
> +	queue_rules_reset_global_params();
>  	while ((rulecat = ast_category_browse(cfg, rulecat))) {
>  		if (!strcasecmp(rulecat, "general")) {
>  			queue_rules_set_global_params(cfg);
> @@ -8795,36 +8801,41 @@ static int reload_queue_rules(int reload)
>  	return AST_MODULE_LOAD_SUCCESS;
>  }
>  
> +/*! Always set the global queue defaults, even if there is no "general" section in queues.conf */
> +static void queue_reset_global_params()
> +{
> +	queue_persistent_members = 0;
> +	autofill_default = 0;
> +	montype_default = 0;
> +	update_cdr = 0;
> +	shared_lastcall = 0;
> +	negative_penalty_invalid = 0;
> +	log_membername_as_agent = 0;
> +}
> +
>  /*! Set the global queue parameters as defined in the "general" section of queues.conf */
>  static void queue_set_global_params(struct ast_config *cfg)
>  {
>  	const char *general_val = NULL;
> -	queue_persistent_members = 0;
>  	if ((general_val = ast_variable_retrieve(cfg, "general", "persistentmembers"))) {
>  		queue_persistent_members = ast_true(general_val);
>  	}
> -	autofill_default = 0;
>  	if ((general_val = ast_variable_retrieve(cfg, "general", "autofill"))) {
>  		autofill_default = ast_true(general_val);
>  	}
> -	montype_default = 0;
>  	if ((general_val = ast_variable_retrieve(cfg, "general", "monitor-type"))) {
>  		if (!strcasecmp(general_val, "mixmonitor"))
>  			montype_default = 1;
>  	}
> -	update_cdr = 0;
>  	if ((general_val = ast_variable_retrieve(cfg, "general", "updatecdr"))) {
>  		update_cdr = ast_true(general_val);
>  	}
> -	shared_lastcall = 0;
>  	if ((general_val = ast_variable_retrieve(cfg, "general", "shared_lastcall"))) {
>  		shared_lastcall = ast_true(general_val);
>  	}
> -	negative_penalty_invalid = 0;
>  	if ((general_val = ast_variable_retrieve(cfg, "general", "negative_penalty_invalid"))) {
>  		negative_penalty_invalid = ast_true(general_val);
>  	}
> -	log_membername_as_agent = 0;
>  	if ((general_val = ast_variable_retrieve(cfg, "general", "log_membername_as_agent"))) {
>  		log_membername_as_agent = ast_true(general_val);
>  	}
> @@ -9128,6 +9139,7 @@ static int reload_queues(int reload, struct ast_flags *mask, const char *queuena
>  
>  	/* Chug through config file. */
>  	cat = NULL;
> +	queue_reset_global_params();
>  	while ((cat = ast_category_browse(cfg, cat)) ) {
>  		if (!strcasecmp(cat, "general") && queue_reload) {
>  			queue_set_global_params(cfg);
> diff --git a/configs/samples/queues.conf.sample b/configs/samples/queues.conf.sample
> index 5c8a80ffee..f1694929fb 100644
> --- a/configs/samples/queues.conf.sample
> +++ b/configs/samples/queues.conf.sample
> @@ -23,7 +23,7 @@ persistentmembers = yes
>  ;    no more available members or no more waiting callers. This is
>  ;    probably more along the lines of how a queue should work and
>  ;    in most cases, you will want to enable this behavior. If you
> -;    do not specify or comment out this option, it will default to yes.
> +;    do not specify or comment out this option, it will default to no.
>  ;
>  ;autofill = no
>  ;
> {noformat}
> Unfortunately, this change will change things for the current users who don't have a {{\[general]}} section.
> So, suggested target for this change?
> - master only?
> - 13 and up, adding an appropriate notice in CHANGES?



--
This message was sent by Atlassian JIRA
(v6.2#6252)



More information about the asterisk-bugs mailing list