<p> Attention is currently required from: Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18008">View Change</a></p><p>7 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/be2bc8d9_778bf31a">Patch Set #2, Line 14:</a> <code style="font-family:monospace,monospace">It is a best-effort request. It works by signaling the</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can you clarify what best-effort really means? Is it guaranteed, or can it not happen?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">It may not happen, because its asynchronous. <br>It may not happen in these cases:<br>1) If the caller is now listening to an annoucement, such as position, it will only leave after this announcement.<br>2) If the caller is now inside a breakout menu, it will only leave after exiting this menu.<br>3) If the caller is already ringing at an extension, it will wait only leave after the ringing ends and the call wasn't answered.</p><p style="white-space: pre-wrap; word-wrap: break-word;">These are limitations because i have no control over the caller's channel, it runs in a different thread. I can only signal it to leave the queue.<br>I could perhaps make the function block, but this will block the AMI connection until #1, #2 or #3 are done.<br>I am not concerned about #1 and #2. I find #3 a little more concerning, but i am not sure how to change this behaviour safely.<br>So this is why i wrote that its a best-effort request.</p><p style="white-space: pre-wrap; word-wrap: break-word;">It is possible to extend the behaviour in the future with a parameter, perhaps something like Async: False, similiar to Originate.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008?tab=comments">Patch Set #4:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">Thanks for the comments. All (hopefully) resolved :-)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File apps/app_queue.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/d3a63896_6a090d4f">Patch Set #2, Line 301:</a> <code style="font-family:monospace,monospace"> </variable></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">The QUEUE_WITHDRAW_INFO dialplan variable should be documented here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/6511eee7_e803cf0c">Patch Set #2, Line 1713:</a> <code style="font-family:monospace,monospace"> char* withdraw_info; /*!< Optional info passed by the caller of QueueWithdrawCaller */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">char *withdraw_info;</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/7e9c4df3_309ccd51">Patch Set #2, Line 7645:</a> <code style="font-family:monospace,monospace"> if (strcmp(ast_channel_name(qe->chan), caller) == 0) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Please review the coding guidelines[1]. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/1c65d282_0bff2ed2">Patch Set #2, Line 7648:</a> <code style="font-family:monospace,monospace"> if(withdraw_info && !qe->withdraw_info) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">In what case would withdraw_info be set already?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">In case another request was sent already.<br>In such case, i thought that its better not to overwrite the existing withdraw info, because perhaps the call is about to leave already due to the first request.<br>If we decide to allow overwriting the withdraw_info, we must free the existing first.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18008/comment/e8de5dc4_bf63f4f8">Patch Set #2, Line 10805:</a> <code style="font-family:monospace,monospace"> switch (request_withdraw_caller_from_queue(queuename, caller, withdraw_info)) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I think from a user perspective this should tell the user if the channel is already in the process o […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ok, i agree.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18008">change 18008</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/18008"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: Ic15aa238e23b2884abdcaadff2fda7679e29b7ec </div>
<div style="display:none"> Gerrit-Change-Number: 18008 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Kfir Itzhak <mastertheknife@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Sun, 13 Feb 2022 12:22:07 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>