[Home]

Summary:ASTERISK-10876: [patch] Add 'voicemail reload' CLI command
Reporter:Eliel Sardanons (eliel)Labels:
Date Opened:2007-11-24 13:13:51.000-0600Date Closed:2007-12-19 14:17:43.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_voicemail.c.patch
Description:Trivial  CLI command to reload voicemail configuration.
Syntax: voicemail reload
Comments:By: Tilghman Lesher (tilghman) 2007-11-25 02:23:16.000-0600

What's wrong with using "reload app_voicemail.so" ?
By: Eliel Sardanons (eliel) 2007-11-25 10:21:41.000-0600

I think that every module that has a .conf associated should has a '* reload' command to retrieve the configuration, there is a question on the dCap exam about this (I think) to confuse you:

How do you reload sip and voicemail without reloading other module?
1 - sip reload and voicemail reload
2 - sip reload and reload app_voicemail.so

They ask you this because they think you will reply with 'sip reload and voicemail reload' because you will think "every command has a reload why voicemail shouldnt?" <-- This dCap question inspire me to add this command.

And I think, we are trying to make asterisk as simple as possible, and not every administrator will know or must know that voicemail functionality is under app_voicemail.so. I also think that we don't loose anything adding this type of commands. I see 'module reload *.so' as a low level command.

I know there is a workaround for reloading voicemail config, but it is hard to remember every command if we don't encapsulate them in a common form.
By: Tilghman Lesher (tilghman) 2007-11-25 10:39:49.000-0600

But that's the point.  SIP also has a similar command:  "module reload chan_sip.so".  In fact, that works for every single module (that has a reload capability), so it's a common command.  Why confuse people by having two commands that do exactly the same thing?
By: Michiel van Baak (mvanbaak) 2007-11-25 10:56:48.000-0600

That means you are going to deprecate 'sip reload' ?
By: Tilghman Lesher (tilghman) 2007-11-25 14:09:10.000-0600

mvanbaak:  I'd prefer that, actually, yes.
By: Eliel Sardanons (eliel) 2007-11-26 10:13:09.000-0600

I agree with Corydon having only one way of doing things... if you want close this issue and we could deprecate all '* reload' cli commands.
By: Tilghman Lesher (tilghman) 2007-11-26 11:22:49.000-0600

Closed on request of reporter.
By: Eliel Sardanons (eliel) 2007-11-27 09:32:43.000-0600

As discussed on asterisk-dev mailing list ('Deprecate every \'* reload\' command?') I reopen this issue for review.
By: Leif Madsen (lmadsen) 2007-12-06 13:43:31.000-0600

We seem to be moving towards a <module> <verb> <argument> format.

What about:

core reload voicemail
core reload sip
core reload iax

etc...
By: Eliel Sardanons (eliel) 2007-12-06 14:32:07.000-0600

blitzrage: I know, but the module here is 'voicemail', your approach is only for the reload module, to move every reload command to the form:
core reload <modulename> (???)
By: Eliel Sardanons (eliel) 2007-12-06 14:33:50.000-0600

You are thinking about a janitor to move every
<module> reload to the form: core reload <module> ?
By: Leif Madsen (lmadsen) 2007-12-06 14:36:53.000-0600

Ya, that'd be the approach I was thinking. Keep it all consistent and put all the reload commands into one place.

You'd still have:

module reload app_voicemail.so

For modules that don't have their own reload command (or 'module [load|unload|reload] app_voicemail.so')
By: Leif Madsen (lmadsen) 2007-12-06 14:38:09.000-0600

I guess I'm not entirely sold on the 'core reload voicemail' change.

1) like you said, it becomes a janitor project

2) voicemail reload still fits with the <module> <verb> <argument> format

In fact... I'm now more convinced you're right and I'm wrong :)

Edit: s/not/now/


By: Eliel Sardanons (eliel) 2007-12-06 14:39:13.000-0600

If this was already discussed and it isn't just an idea, I could start working on this... please let me know if this was discussed and all developers agree.
By: Leif Madsen (lmadsen) 2007-12-06 14:41:41.000-0600

The 'core reload foo' approach was just an idea from me.. and I'm not even convinced its a good idea anymore :)
By: Eliel Sardanons (eliel) 2007-12-06 14:41:48.000-0600

Ok, so, this command could be commited.
I am waiting for Tilghman to commit this and I will start working on the other modules that don't have the
{<module> reload}  command and they get configuration from a config file (if there are more (??))
By: Leif Madsen (lmadsen) 2007-12-06 14:46:30.000-0600

Yes, my recommendation now would be to commit this patch and finish up any additional places where this functionality may be missing, or implemented inconsistently.

Please see bug 8925 (which I've marked as related) for any additional patches related to CLI command consistencies.

Thanks!
By: Digium Subversion (svnbot) 2007-12-19 14:17:43.000-0600

Repository: asterisk
Revision: 94053

U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r94053 | tilghman | 2007-12-19 14:17:42 -0600 (Wed, 19 Dec 2007) | 5 lines

Add 'voicemail reload' command.
Reported by: eliel
Patch by: eliel
(Closes issue ASTERISK-10876)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=94053