Sounds good! Sent from phone, thus brief. Am 17.11.2014 10:53 schrieb "singh.janmejay" <[email protected]>:
> On Mon, Nov 17, 2014 at 3:11 PM, Rainer Gerhards <[email protected] > > > wrote: > > > 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. > > > The foreach patch needs a 'reset' kind of control to make the loop > implementation useful. Sounds like we can go with 'reset' for now (so I can > complete the patch) and once you have had time to re-look at the > merge/leaf-node special behaviour, we can either let 'reset' live(and keep > set unchanged), or kill it after making 'set' identical to it (if it > doesn't have any undesirable effects). > > So shall I go ahead and implement 'reset'? > > > > 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. > > > > > > -- > 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.
