Summary:	ASTERISK-10060: Allow realtime members on non realtime queues/A member with priorty < 0 is logged out
Reporter:	Gregory Hinton Nietsky (irroot)	Labels:
Date Opened:	2007-08-10 02:18:10	Date Closed:	2007-10-31 10:27:30
Priority:	Trivial	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_queue
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) app_queue.c.patch ( 1) asterisk-1.4.10.patch.queue ( 2) realtime_pause.patch ( 3) realtime_pause2.patch
Description:	hi there some trivial patches extending on the recent changes (adding update_realtime_members) it seems to make logic sence to allow realtime agents on non realtime queues (ive been doing this for over a year already). ive also been setting the priorty field in SQL to a negitive value as opposed to deleteing the agent to log them off and then back to there existing value to log them on again.
Comments:	By: Leif Madsen (lmadsen) 2007-08-10 09:59:50 I guess I can kinda see where you're going with the setting the priority field to a negative value in the database, but to do this I just have a table with my members and create a view in the database, then point the queue_members in extconfig.conf to the view. The view then only lists the members that have the 'logged_on' flag set in the primary table. I think that functionality could be confusing to people, so I'd rather see it live outside in the database, and not in Asterisk. The realtime members being used in a non-realtime queue I would agree with though (didn't realize that didn't work) By: Mark Michelson (mmichelson) 2007-08-10 10:01:28 I agree with blitzrage 100% on this. By: Gregory Hinton Nietsky (irroot) 2007-08-10 10:43:56 can see the logic of leaving it outside asterisk and it does change the way it has worked till now ... i dont think it has ever worked ... dynamic members persistant in astdb did work but was a nasty hack so i put the support into the app where it belongs ... the patch is now much more tidy thx to the new function ... but does the same thing as my older convoluted patch. By: Digium Subversion (svnbot) 2007-08-13 10:22:04 Repository: asterisk Revision: 79238 ------------------------------------------------------------------------ r79238 | mmichelson | 2007-08-13 10:22:03 -0500 (Mon, 13 Aug 2007) | 5 lines Allow non-realtime queues to have realtime members (issue ASTERISK-10060, reported and patched by irroot) ------------------------------------------------------------------------ By: Mark Michelson (mmichelson) 2007-08-13 10:23:12 I added the part regarding allowing non-realtime queues to use realtime queues. Thanks for adding this capability! I'm still a bit wary about the other part of the patch since I think it would cause unexpected behavior for a lot of people. By: Gregory Hinton Nietsky (irroot) 2007-08-13 12:17:19 pleasure all mine ... it was a nice little hack to integrate with the GUI to log users in and out ... by changing the sign of there penalty. i would like to see updates to the realtime DB when a user is paused (autopause) and updateing a view is not possible with all backends ... finding a way to determine if a memeber is logged in or not from the query would be a worthwile venture prehaps add a "loggedout" option to the schema that if present will not load the member. ... By: Leif Madsen (lmadsen) 2007-08-13 12:31:14 Do you mean like a SQL query to check the database? Because you could utilize func_odbc for that purpose. By: Gregory Hinton Nietsky (irroot) 2007-08-13 12:42:40 yeah sure im doing that but there is one place it is lacking ... when a member is auto paused it should be writen back as this happens inside the app it should be updated inside the app. this seems to be the only change made inside the app that should be writen back IMHO. the behavior is not what is expected in this case when there is a reload. By: Leif Madsen (lmadsen) 2007-08-13 13:07:22 Ahhhh gotcha. I'm interested in putnopvuts comments on this. By: Mark Michelson (mmichelson) 2007-08-20 09:45:09 Sorry to take so long to get back around to this. If I understand correctly, you're saying that if a realtime member is paused and a reload is issued, the member is no longer paused? If that's the case, then it should be fixed. I'll get to it. By: Gregory Hinton Nietsky (irroot) 2007-08-20 10:23:17 its fine no stress ... as the the pause status is not stored for realtime users (persistantmembers off and not dynamicialy added) for a auto pause there is no write back to set the status a reload will loose this .... what i sugest is a 2 prong approach 1)auto pause is tempory for say XX seconds ... (i prefer this approach as it will allow a call to escalate to a higher penalty caller ... and have the agent ring next time round) (set autopause to a integer) 2)as is (if autopause set to yes) the user is permanently paused in both cases esp 2 the DB needs to be updated to reflect this ... now using a view in pg at least you cannot update it so the table used by queues should not be a view ... and there should be a way of determining the loged in/out status from the query .... hope this makes some sense ... By: Digium Subversion (svnbot) 2007-08-20 16:21:19 Repository: asterisk Revision: 80086 ------------------------------------------------------------------------ r80086 | mmichelson | 2007-08-20 16:21:19 -0500 (Mon, 20 Aug 2007) | 5 lines After a discussion on #asterisk-dev, it was decided that this should be in 1.4 as well. (issue ASTERISK-10060, reported and patched by irroot) ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-08-20 16:24:13 Repository: asterisk Revision: 80087 ------------------------------------------------------------------------ r80087 | mmichelson | 2007-08-20 16:24:12 -0500 (Mon, 20 Aug 2007) | 12 lines Blocked revisions 80086 via svnmerge ........ r80086 | mmichelson | 2007-08-20 16:39:17 -0500 (Mon, 20 Aug 2007) | 5 lines After a discussion on #asterisk-dev, it was decided that this should be in 1.4 as well. (issue ASTERISK-10060, reported and patched by irroot) ........ ------------------------------------------------------------------------ By: Mark Michelson (mmichelson) 2007-08-20 17:04:47 After discussing things on #asterisk-dev, it was decided to go ahead and put the capability of having realtime members for static queues in 1.4 as well as trunk. After looking into things a bit more, I realize why it is that nothing ever gets written to realtime in app_queue. The reason is that members are identified by both the queue they belong to and their interface. The realtime API only allows one keyfield to be used when updating something. Because of this, there are two ways around this. 1. Change the API to accommodate multi-columnn primary keys. This would not be trivial since it would mean potentially changing a lot of existing code to use this new API. 2. Force users of realtime members to use a single unique identifier in their table to identify a specific member of a specific queue (similar to what realtime voicemail requires). This would require no change to the API but would require that users of queues with realtime members possibly change their table structure. Using #2 is my preference since it is less invasive than #1 and since we already have a precedent (realtime voicemail) we should stick with this. That being said, whatever patch I put up here will be written under the assumption that the user has defined such a unique id field. Details will come after I've written the patch. By: Leif Madsen (lmadsen) 2007-08-21 08:14:56 irroot: I've deleted your note because all patches / changes to code should be uploaded via a unified diff in order to control the licensing of code in the tracker. By: Gregory Hinton Nietsky (irroot) 2007-08-21 08:54:35 hi there here is my auto logout write to db ..  (asterisk-1.4.10.patch.queue) i have a customer that wanted auto logout ... auto pause makes more sense ... it looks up the user finds the id and updates it ... simple ... By: Mark Michelson (mmichelson) 2007-08-21 10:35:58 There are a couple of problems with your patch. 1. The if(rtbid) is unnecessary. The while loop will take care of this. 2. There is a possibility for a crash. If the while loop finishes without finding the field "id" then the if(!ast_strlen_zero(rtbid->value)) will cause a segfault since rtbid will be NULL. By: Mark Michelson (mmichelson) 2007-08-21 13:28:08 I've uploaded a patch (realtime_pause.patch) which takes care of writing the paused status of a member to realtime. It is similar to your earlier patch (asterisk-1.4.10.patch.queue) except that I made a generic function to update a field for a realtime queue member. You'll see what I mean when you look at the patch. Using this function I uploaded as a base, it will be trivial to update whatever fields for a member we wish to update. The eventual goal will be to persist data in exactly the same way that is done in the astdb if you used persistentmembers=yes. This way, a reload will not cause problems. Edit: I forgot to mention a brief warning. This patch assumes that you have a field for your queue members called "uniqueid" which uniquely identifies a member and a field called "paused" which determines if the member is paused or not. By: Gregory Hinton Nietsky (irroot) 2007-08-22 01:35:25 thank you for your input looks good ... By: Mark Michelson (mmichelson) 2007-08-22 10:35:26 I realized I had made a small mistake in my realtime_pause.patch. The problem is that I misinterpreted the return value of ast_update_realtime(). realtime_pause2.patch should correct this. By: Digium Subversion (svnbot) 2007-08-29 11:17:19 Repository: asterisk Revision: 81349 ------------------------------------------------------------------------ r81349 | mmichelson | 2007-08-29 11:17:17 -0500 (Wed, 29 Aug 2007) | 12 lines This patch, in essence, will correctly pause a realtime queue member and reflect those changes in the realtime engine. (issue ASTERISK-10060, reported by irroot, patch by me) This patch creates a new function called update_realtime_member_field, which is a generic function which will allow any one field of a realtime queue member to be updated. This patch only uses this function to update the paused status of a queue member, but it lays the foundation for persisting the state of a realtime member the same way that static members' state is maintained when using the persistentmembers setting ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-08-29 11:21:29 Repository: asterisk Revision: 81350 ------------------------------------------------------------------------ r81350 | mmichelson | 2007-08-29 11:21:28 -0500 (Wed, 29 Aug 2007) | 20 lines Merged revisions 81349 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r81349 | mmichelson | 2007-08-29 11:35:29 -0500 (Wed, 29 Aug 2007) | 12 lines This patch, in essence, will correctly pause a realtime queue member and reflect those changes in the realtime engine. (issue ASTERISK-10060, reported by irroot, patch by me) This patch creates a new function called update_realtime_member_field, which is a generic function which will allow any one field of a realtime queue member to be updated. This patch only uses this function to update the paused status of a queue member, but it lays the foundation for persisting the state of a realtime member the same way that static members' state is maintained when using the persistentmembers setting ........ ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-08-29 12:54:35 Repository: asterisk Revision: 81352 ------------------------------------------------------------------------ r81352 | murf | 2007-08-29 12:54:32 -0500 (Wed, 29 Aug 2007) | 128 lines Merged revisions 81326,81332-81335,81341,81343-81345,81347-81348,81350 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r81326 | file | 2007-08-28 20:21:08 -0600 (Tue, 28 Aug 2007) | 2 lines Add inline function for signed linear subtraction. ................ r81332 | file | 2007-08-29 08:16:07 -0600 (Wed, 29 Aug 2007) | 12 lines Merged revisions 81331 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r81331 | file | 2007-08-29 11:13:55 -0300 (Wed, 29 Aug 2007) | 4 lines (closes issue ASTERISK-9405) Reported by: mattv Make rtp timeouts work even if two RTP streams are directly bridged in the RTP stack. ........ ................ r81333 | mmichelson | 2007-08-29 08:19:33 -0600 (Wed, 29 Aug 2007) | 5 lines Changing a NOTICE to a DEBUG. (closes issue ASTERISK-10190, reported and patched by junky, with small modification by me) ................ r81334 | file | 2007-08-29 09:19:11 -0600 (Wed, 29 Aug 2007) | 2 lines Add API calls for iterating through an event. This should allow events to have multiple information elements (while there was nothing preventing it before you could not actually access any except the first one). ................ r81335 | tilghman | 2007-08-29 09:21:10 -0600 (Wed, 29 Aug 2007) | 2 lines Changed one too many variable settings in issue ASTERISK-9044 (closes issue ASTERISK-10191) ................ r81341 | mmichelson | 2007-08-29 09:57:27 -0600 (Wed, 29 Aug 2007) | 16 lines Merged revisions 81340 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r81340 | mmichelson | 2007-08-29 10:52:42 -0500 (Wed, 29 Aug 2007) | 8 lines This fix creates a more accurate way of detecting whether realtime members were deleted. (closes issue 10541, reported by Alric, patched by me) The REALLY nice things about this patch is that queue members now have a "realtime" field which will be true if the member is a realtime member. This means we can check this value prior to certain processing if it should ONLY be done for realtime members. ........ ................ r81343 | russell | 2007-08-29 09:59:10 -0600 (Wed, 29 Aug 2007) | 11 lines Merged revisions 81342 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r81342 | russell | 2007-08-29 10:57:29 -0500 (Wed, 29 Aug 2007) | 3 lines If chan_h323 is not being built, don't use g++ to do the final link of Asterisk. (in response to a question on the asterisk-dev list) ........ ................ r81344 | file | 2007-08-29 10:03:51 -0600 (Wed, 29 Aug 2007) | 2 lines To keep others happy... revert part of my additions so trunk works. ................ r81345 | file | 2007-08-29 10:07:35 -0600 (Wed, 29 Aug 2007) | 2 lines This concludes bringing trunk back to a working state. ................ r81347 | mmichelson | 2007-08-29 10:09:02 -0600 (Wed, 29 Aug 2007) | 11 lines Merged revisions 81346 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r81346 | mmichelson | 2007-08-29 11:08:09 -0500 (Wed, 29 Aug 2007) | 3 lines Changed some tabs to spaces ........ ................ r81348 | file | 2007-08-29 10:25:30 -0600 (Wed, 29 Aug 2007) | 2 lines Return ast_event_get_ie_raw to using an iterator and fix logic in ast_event_iterator_next. ................ r81350 | mmichelson | 2007-08-29 10:39:40 -0600 (Wed, 29 Aug 2007) | 20 lines Merged revisions 81349 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r81349 | mmichelson | 2007-08-29 11:35:29 -0500 (Wed, 29 Aug 2007) | 12 lines This patch, in essence, will correctly pause a realtime queue member and reflect those changes in the realtime engine. (issue ASTERISK-10060, reported by irroot, patch by me) This patch creates a new function called update_realtime_member_field, which is a generic function which will allow any one field of a realtime queue member to be updated. This patch only uses this function to update the paused status of a queue member, but it lays the foundation for persisting the state of a realtime member the same way that static members' state is maintained when using the persistentmembers setting ........ ................ ------------------------------------------------------------------------ By: Mark Michelson (mmichelson) 2007-10-31 10:27:29 I had meant to close this issue out earlier but apparently forgot about it.