[asterisk-dev] [Code Review] 3148: res_pjsip: Config option to enable PJSIP logger at load time

[Kevin Harwell]

> On Jan. 21, 2014, 7:14 p.m., Paul Belanger wrote:
> > branches/12/contrib/ast-db-manage/config/versions/2fc7930b41b3_add_pjsip_endpoint_options_for_12_1.py, line 1
> > <https://reviewboard.asterisk.org/r/3148/diff/1/?file=53005#file53005line1>
> >
> >     You should be creating new alembic scripts not modifying committed ones. 
> >     
> >     This is going to breaks peoples systems that have already invoked the scripts.
> 
> Russell Bryant wrote:
>     right.  each script should stay unmodified.  any db schema changes should be a new script, which is a migration from the old schema to the updated one.  That way at each release, people can re-run alembic to apply all of the updates since the version of the schema they currently have.
> 
> Kevin Harwell wrote:
>     Correct and usually we would do just as mentioned.  In this case though the modified script has not been released (will be in 12.1).  Once released then it will stay unmodified and changes will go into a new script.
> 
> Paul Belanger wrote:
>     Well, the issue being you break peoples system that run the TIP of /branches/12, which you cannot control who as run the script.  So, the ideal way to handle it, is just leave the file as is and run 'alembic revision -m 'enable pjsip debug'.  This creates[1] fresh alembic scripts which you can move this code into, then you just build the upgrade / downgrade logic around it.
>     
>     Once the script has been committed to subversion, you have to assume somebody has run it, modifying it after the fact breaks these systems.
>     
>     [1] http://alembic.readthedocs.org/en/latest/tutorial.html#running-our-second-migration

Agreed, and that is how I originally started out when addressing the addition of the "debug" option and would have done just that except unfortunately the "...options_for_12_1" script would fail as it was.  It was trying to add a column to a table that didn't exist.  It also had a problem with previously created enums when running against postgres.

Since the script contains bugs it had to be modified and since it hasn't been officially released it made it easier to do so for this case.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3148/#review10653
-----------------------------------------------------------


On Jan. 21, 2014, 5:01 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3148/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2014, 5:01 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23038
>     https://issues.asterisk.org/jira/browse/ASTERISK-23038
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Added a "debug" configuration option for res_pjsip that when set to "yes" enables SIP messages to be logged.  It is specified under the "system" type.
> 
> Also updated the alembic 12.1 script to include this option as well as a few others that were missing.  Also updated the "_adding_extensions" script in order to make the "id" column on the table a primary key because mysql needed it to be as such.
> 
> 
> Diffs
> -----
> 
>   branches/12/res/res_pjsip_logger.c 405906 
>   branches/12/res/res_pjsip/config_system.c 405906 
>   branches/12/res/res_pjsip.c 405906 
>   branches/12/include/asterisk/res_pjsip.h 405906 
>   branches/12/contrib/ast-db-manage/config/versions/581a4264e537_adding_extensions.py 405906 
>   branches/12/contrib/ast-db-manage/config/versions/2fc7930b41b3_add_pjsip_endpoint_options_for_12_1.py 405906 
>   branches/12/configs/pjsip.conf.sample 405906 
> 
> Diff: https://reviewboard.asterisk.org/r/3148/diff/
> 
> 
> Testing
> -------
> 
> Set the "debug" option in the pjsip.conf file and observed SIP debug messages on the console.  Also, tested the modified alembic scripts against postgres and mysql database servers.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140122/393cbe81/attachment-0001.html>