[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