----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4033/#review13428 -----------------------------------------------------------
Ship it! My "Ship it!" is a cautious one. Basically, the unit tests are good evidence to me that things are working as expected, but when changes are being made to such a fundamental area of code as the config API, I feel like a second "ship it!" from someone else should occur before actually committing. Also, I think a discussion needs to occur with regards to this patch's inclusion in Asterisk 12. branches/12/tests/test_config.c <https://reviewboard.asterisk.org/r/4033/#comment23952> For all of the snprintf() calls in this test, use sizeof(temp) instead of 32. - Mark Michelson On Sept. 30, 2014, 9:24 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4033/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2014, 9:24 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > This patch provides the capability to manipulate templates and categories > with non-unique names via AMI. > > Summary of changes: > > GetConfig and GetConfigJSON: > Added "Filter" parameter: A comma separated list of name_regex=value_regex > expressions which will cause only categories whose variables match all > expressions to be considered. The special variable name TEMPLATES can be > used to control whether templates are included. Passing 'include' as the > value will include templates along with normal categories. Passing 'restrict' > as the value will restrict the operation to ONLY templates. Not specifying a > TEMPLATES expression results in the current default behavior which is to not > include templates. > > Examples: > "GetConfig?Filename=pjsip.conf&Category=myitsp&Filter=type=aor" would return > only 'myitsp' categories with a type of 'aor'. > > "GetConfig?Filename=pjsip.conf&Category=itsp-template&Filter=TEMPLATES=restrict,type=aor" > would return only 'itsp-template' categories that are actually templates > with a type of 'aor'. > > "GetConfig?Filename=pjsip.conf&Filter=type=registration,endpoint=myitsp" > would return only registrations whose corresponding endpoint is 'myitsp'. > > The output from GetConfig and GetConfigJSON also includes variables > indicating if the returned category is a template and the template names a > category inherits from if any. > > UpdateConfig: > NewCat now includes options for allowing duplicate category names, indicating > if the category should be created as a template, and specifying templates the > category should inherit from. The rest of the actions now accept a filter > string as defined above. If there are non-unique category names, you can now > update specific ones based on variable values. > > To facilitate the new capabilities in manager, corresponding changes had to > be made to config, most notably the addition of filter criteria to many of > the APIs. In some cases it was easy to change the references to use the new > prototype but others would have required touching too many files for this > patch so a wrapper with the original prototype was created. Macros couldn't > be used in this case because it would break binary compatibility with modules > such as res_digium_phone that are linked to real symbols. > > > Diffs > ----- > > branches/12/tests/test_sorcery_realtime.c 424175 > branches/12/tests/test_sorcery.c 424175 > branches/12/tests/test_config.c 424175 > branches/12/res/res_sorcery_realtime.c 424175 > branches/12/res/res_sorcery_config.c 424175 > branches/12/pbx/pbx_realtime.c 424175 > branches/12/main/manager.c 424175 > branches/12/main/config.c 424175 > branches/12/include/asterisk/config.h 424175 > branches/12/apps/app_voicemail.c 424175 > branches/12/apps/app_directory.c 424175 > > Diff: https://reviewboard.asterisk.org/r/4033/diff/ > > > Testing > ------- > > Testsuite before and after runs gave the same results. > A set of unit tests for config was added to test_config. They test basic, > filtered and template operations. > > > Thanks, > > George Joseph > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
