2014-11-17 10:35 GMT+01:00 singh.janmejay <[email protected]>:
> So for users that do not want tree merging, is 'reset' the right answer? > > Of the form: > > reset $.foo = $!bar; > > As opposed to set, this will simply remove whatever value is present at foo > and set the given value in. > > I guess so. But quite honestly, I would really need to read the code and commits in detail. That past couple of weeks I've been building VMs and working on testbench automation, so I really do not have this on my radar right now. So unfortunately it's nothing I can answer right out of my head. I also admit that I do not want to leave the testbench area right now, as I have invested so much time into it and breaking away from it can waste a lot of this time. The general story is this: we needed to implement variables in a tree-like manner. There have been a couple of choices at those times (among them was a part of ding-libs, various json libs, "do it yourself" implementation). For simplicity, we settled on json (and I think switched libs to json-c a bit later). If we now reset containers, each set affect all other variables in that container, which would not make much sense. In that light, an explicit reset sounds like the right thing to do. But again, I would really need some more time to look at this and the implications. I am also not sure if the current tests capture all side-effects possible ... simply because I did not think about this when I implemented and wrote the tests (did sombody mention that it is a bad thing if a single person implements, writes tests and documents? ;)). Hope that helps at least a bit. Rainer > On Mon, Nov 17, 2014 at 2:58 PM, Rainer Gerhards <[email protected] > > > wrote: > > > I need to review this in detail. IIRC, this is required by some modules. > > That "set" can manipulate trees is more or less a side-effect of the > > variable implementation via json-c. Replacing json-c with something that > > offers more performance is on the todo list for quite a while. > > > > Rainer > > > > 2014-11-17 10:23 GMT+01:00 singh.janmejay <[email protected]>: > > > > > Bump. > > > > > > On Thu, Nov 13, 2014 at 12:13 PM, singh.janmejay < > > [email protected] > > > > > > > wrote: > > > > > > > Also, tests seem to pass with the change I mentioned above (set > > replacing > > > > the contents of field regardless of it being object, leaf or null). > > > > > > > > > > > > > > ============================================================================ > > > > Testsuite summary for rsyslog 8.5.0 > > > > > > > > > > ============================================================================ > > > > # TOTAL: 125 # PASS: 120 # SKIP: 5 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # > > > ERROR: > > > > 0 > > > > > > > > > > ============================================================================ > > > > > > > > > > > > On Thu, Nov 13, 2014 at 12:11 PM, singh.janmejay < > > > [email protected] > > > > > wrote: > > > > > > > >> Removing all the cases and replacing everything with just: > > > >> > > > >> json_object_object_add(parent, (char*)leaf, json); > > > >> > > > >> Changes the semantics to always replace the value, regardless of it > > > being > > > >> object, leaf or null. > > > >> > > > >> From the pov of set x = y, it seems like the right thing to do. > > > >> > > > >> But im sure the existing implementation is the way it is for a > reason. > > > >> > > > >> In case its just a bug, should we go ahead with this replacement? > (all > > > >> cases removed and just one simple object_add call, no dereference of > > old > > > >> value required either). > > > >> > > > >> In case its not a bug, what about having another statement (reset?) > > for > > > >> this purpose? > > > >> > > > >> So, if user wants objects to be merged and object not be be replaced > > > with > > > >> leaf etc, they can use 'set $.foo = $.bar;' but if they want > > > no-conditions > > > >> replace-whatever semantics, they can use 'reset $.foo = $.bar;'. > > > >> > > > >> Ideally, may be we should call set something else, merge? but that > may > > > be > > > >> bad from backward compatibility pov. > > > >> > > > >> On Thu, Nov 13, 2014 at 11:07 AM, singh.janmejay < > > > >> [email protected]> wrote: > > > >> > > > >>> Hi Rainer, > > > >>> > > > >>> Im talking about msgAddJSON. Quickly glanced through your > > > >>> commit(71a5122fa), but it doesn't seem to talk much about it. Why > do > > we > > > >>> disallow replacing an object with a non-object value? > > > >>> > > > >>> Also, not really the same issue, but another clarification in the > > same > > > >>> area of code. Why do we merge objects when user has called 'set'? > > > shouldn't > > > >>> we replace old json_object with new one? > > > >>> > > > >>> -- > > > >>> Regards, > > > >>> Janmejay > > > >>> http://codehunk.wordpress.com > > > >>> > > > >> > > > >> > > > >> > > > >> -- > > > >> Regards, > > > >> Janmejay > > > >> http://codehunk.wordpress.com > > > >> > > > > > > > > > > > > > > > > -- > > > > Regards, > > > > Janmejay > > > > http://codehunk.wordpress.com > > > > > > > > > > > > > > > > -- > > > Regards, > > > Janmejay > > > http://codehunk.wordpress.com > > > _______________________________________________ > > > rsyslog mailing list > > > http://lists.adiscon.net/mailman/listinfo/rsyslog > > > http://www.rsyslog.com/professional-services/ > > > What's up with rsyslog? Follow https://twitter.com/rgerhards > > > NOTE WELL: This is a PUBLIC mailing list, posts are ARCHIVED by a > myriad > > > of sites beyond our control. PLEASE UNSUBSCRIBE and DO NOT POST if you > > > DON'T LIKE THAT. > > > > > _______________________________________________ > > rsyslog mailing list > > http://lists.adiscon.net/mailman/listinfo/rsyslog > > http://www.rsyslog.com/professional-services/ > > What's up with rsyslog? Follow https://twitter.com/rgerhards > > NOTE WELL: This is a PUBLIC mailing list, posts are ARCHIVED by a myriad > > of sites beyond our control. PLEASE UNSUBSCRIBE and DO NOT POST if you > > DON'T LIKE THAT. > > > > > > -- > Regards, > Janmejay > http://codehunk.wordpress.com > _______________________________________________ > rsyslog mailing list > http://lists.adiscon.net/mailman/listinfo/rsyslog > http://www.rsyslog.com/professional-services/ > What's up with rsyslog? Follow https://twitter.com/rgerhards > NOTE WELL: This is a PUBLIC mailing list, posts are ARCHIVED by a myriad > of sites beyond our control. PLEASE UNSUBSCRIBE and DO NOT POST if you > DON'T LIKE THAT. > _______________________________________________ rsyslog mailing list http://lists.adiscon.net/mailman/listinfo/rsyslog http://www.rsyslog.com/professional-services/ What's up with rsyslog? Follow https://twitter.com/rgerhards NOTE WELL: This is a PUBLIC mailing list, posts are ARCHIVED by a myriad of sites beyond our control. PLEASE UNSUBSCRIBE and DO NOT POST if you DON'T LIKE THAT.
