----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3305/#review11103 -----------------------------------------------------------
/branches/12/main/sorcery.c <https://reviewboard.asterisk.org/r/3305/#comment20755> This is still a list container. You need to provide a hash callback and multiple buckets for it to not be a list container. How about using the nice OBJECT_FIELD_BUCKETS define you just created here? /branches/12/main/sorcery.c <https://reviewboard.asterisk.org/r/3305/#comment20756> You should assert this to require it to be non-NULL. /branches/12/res/res_sorcery_realtime.c <https://reviewboard.asterisk.org/r/3305/#comment20757> Skip this if object_type doesn't exist. Currently if object_type doesn't exist, you remove everything. This may be good or bad. Good in that nothing will be configured. Bad in that everything is filtered out. - rmudgett On March 6, 2014, 11:27 a.m., Mark Michelson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3305/ > ----------------------------------------------------------- > > (Updated March 6, 2014, 11:27 a.m.) > > > Review request for Asterisk Developers and Joshua Colp. > > > Repository: Asterisk > > > Description > ------- > > When testing out realtime for PJSIP, I encountered a problem. In my database, > I had defined additional columns beyond the ones that Asterisk cared about. > As a result, when data was pulled from the database, the config framework > barfed, saying it didn't know what these object fields were. For config > files, this practice is fantastic. It makes sure to raise a giant red flag to > people that they are attempting to configure something that is not recognized > by Asterisk. For realtime, on the other hand, it is legitimate for a database > to contain supplemental data alongside the data Asterisk cares about. > > What I have done is to modify res_sorcery_realtime.c to filter out fields > from the retrieved objectset that have not been registered with sorcery. In > addition, I have created two realtime sorcery unit tests. The first one > simply creates data in a realtime backend and then has sorcery atttempt to > retrieve the data, ensuring that an object is allocated with the expected > data. The second test is similar, except that in addition to the data that > has been registered with sorcery, an additional field is created in the > realtime backend. This test ensures that the data can still be retrieved and > successfully allocated by sorcery. > > Of all things in this review, the thing I'm concerned about most is the log > message that has been added to indicate that an object has been filtered out > of the resultset retrieved from realtime. I initally wanted to make this a > NOTICE-level message, but I was afraid that with the frequency with which > objects are retrieved from realtime, this could spam the logs a little too > much. On the other hand, leaving it as a level 1 debug message I have now > could make it difficult for someone that has misconfigured their database > (say they accidentally have a typo in a column name) from seeing that data > they expect to be relevant is being ignored by Asterisk. Any suggestions on > how to handle this? > > > Diffs > ----- > > /branches/12/tests/test_sorcery_realtime.c 410010 > /branches/12/res/res_sorcery_realtime.c 410010 > /branches/12/main/sorcery.c 410010 > /branches/12/include/asterisk/sorcery.h 410010 > > Diff: https://reviewboard.asterisk.org/r/3305/diff/ > > > Testing > ------- > > All realtime sorcery unit tests pass with flying colors. > > > Thanks, > > Mark Michelson > >
-- _____________________________________________________________________ -- 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