-
Notifications
You must be signed in to change notification settings - Fork 1
vanstyn frew irc chat 2013 01 25
vanstyn edited this page Jan 25, 2013
·
1 revision
<vanstyn> frew: Ok... So what I have done is started using the GitHub wiki as the place to record documentation and to brainstorm questions/todos
<vanstyn> https://github.com/vanstyn/DBIx-Class-AuditAny/wiki
<frew> k, where should we start?
<frew> lead teh way :)
<vanstyn> I created a page specirfically for the Shadow integration: https://github.com/vanstyn/DBIx-Class-AuditAny/wiki/Shadow_Collector
<vanstyn> but, the first place I'd like to start is feedback on this: https://github.com/vanstyn/DBIx-Class-AuditAny/wiki/API
<frew> vanstyn: so really
<frew> it's not attached to the schema
<frew> but it has a schema
<frew> so really the schema is attached to it?
<vanstyn> Conceptually it is attached to the schema... it is one of any number of "Auditors" that are attached to the schema
<frew> I mean if I have a schema object
<frew> can I get the auditors frmo it?
<vanstyn> now, it does do this under the good by modifying the classes with CLass::MOP
<vanstyn> yes
<vanstyn> $schema->auditors
<frew> interesting
<frew> so basically
<frew> you apply a runtime role or something
<ilmari> keep them away from chocolate
<frew> and then $schema->add_auditor or w/e
<vanstyn> anyway, as I was saying, it does modify the class... but only once, and then adds modifiers gobally that iterate over the attached auditors
<_Dave> "might_have/has_one" must not be on columns with is_nullable set to true
<_Dave> arse
<vanstyn> yes,, we could do a API that adds it from the schema side just as well
<vanstyn> via a $schema->add_auditor
<frew> vanstyn: does it modify the class or create an anonymous class?
<frew> vanstyn: if it modifies the class it is basically just a surprising component
<vanstyn> look at line 259 in https://github.com/vanstyn/DBIx-Class-AuditAny/blob/master/lib/DBIx/Class/AuditAny.pm
<vanstyn> and at my comment starting on line 297
<frew> ok
<frew> so as far as I can tell it *does* mutate the original class
<vanstyn> yes
<frew> which is kinda silly
<vanstyn> so what you are going to say is we do it via a add_auditor
<frew> well no
<frew> I was gonna say if you want to apply it to an object
<frew> you should create a new anon class for that object and mutate that
<frew> what if I have two schemata connected to sep db's?
<vanstyn> ah ha
<frew> and only want to audit one?
<frew> but that's just an impl detail
<vanstyn> well, I relay on logic within the modified method to iterate and then call code only for its specific auditors
<vanstyn> but I can see how it could be considered "spooky action at a distance"
<frew> making an anon class is what I recommend at this point at attachment time, that won't hurt your stuff and won't mutate the schema class but just the object, effecitvely, which is what you want
<frew> comment #2
<frew> you only track changes frmo txn_do ?
<frew> if so, what about txn_scope_guard?
<vanstyn> no, I track all changes
<frew> ok, then I misunderstood something, will keep reading
<vanstyn> well, that is another item.... I am doing stuff that I think is reinventing txn_scope_guard
<frew> "That functionality also makes AuditAny a nice/easy debug tool as various kinds of tracking can be thrown on a db with minimal interference with the rest of the code and setup time. "
<frew> that's a cool idea
<vanstyn> thanks
<frew> "what part of this freaking test caused this to change?! Oh turn on auditing and get a stack trace for each change"
<frew> very clever
<vanstyn> exactly
<frew> vanstyn: so why are you reinventing txn_scope_guard?
<vanstyn> Well, I *think* I am and its because I don't understand it, and as I started to understand it I began to relaized that I had probably reinventied it
<vanstyn> parts of it
<vanstyn> so I want to learn about that and switch to using that
<frew> well it's just a different, lighter interface to txn's
<vanstyn> here is my basic flow:
<vanstyn> as soon as a txn_do is called, a changeset is started, unless a changeset has already been started
<frew> hm
<frew> shadow does that basically
<frew> but shadow is a component
<frew> and iirc it has changeset_do
<frew> whcih you can pass args to, and the changesset is an object, which defaults to having a user and session column, so you can say who did what
<vanstyn> if an insert/update/delete happens outside a changeset, a changeset is started that will consiste of the single change
<frew> right
<frew> I think shadow does the exact same thing
<vanstyn> right, I think so
<frew> (compared to Journal which just lost non txn'd changes iirc)
<vanstyn> and about the user/session columns in the changeset, here was my idea on that
<vanstyn> datapoints are defined which can be anything you want. maybe a session id, maybe a user, maybe an ip address... anything you want
<frew> so you serialize a hash or something?
<vanstyn> so I have this concept of "datapoints" which belong to a "context"
<frew> oohh
<frew> so it's like, two tables
<frew> and datapoints are k/v
<vanstyn> no, I use CodeRefs, and they are called at the appropraite time based on their context
<vanstyn> yes
<frew> ok
<frew> that's cool
<frew> so what if I'm a crufty db admin and don't want that
<frew> it feasible to support either?
<frew> because when I'm testing, the datapoints thing is obviously superior
<vanstyn> well, that is why there are tons of built-in datapoints
<frew> but when I'm doing permanent auditing I don't really care about as many types of data; just who did what, when
<frew> I mean making context a single table instead of related datapoints, for permanent usage
<vanstyn> right, that is what the Collector does.... it organizes the datapoints and stores them in some logical way
<frew> ok, so we can override the collector and do it in a single row if needed
<frew> that' sjust not the default, yes?
<vanstyn> sure
<vanstyn> there is no "default"
<frew> ok.
<vanstyn> there are different sets of supplied collectors and datapoints
<vanstyn> so, for instance, I have the collector DBIC
<frew> sure
<frew> shadow is a collector right?
<frew> well, coudl be
<vanstyn> well,
<vanstyn> right
<frew> and datapoints is *what* you log?
<vanstyn> In the case of shadow, you could also think of the Collector as a *connector*
<frew> so by default shadow logs the datapoints user and session, but you could have a stacktrace datapoint?
<vanstyn> datapoints are what is made available to the collector
<vanstyn> and then the collector does what it wants with them
<arcanez> frew: looks like weechat just updated.
<frew> arcanez: cool :)
<frew> vanstyn: you lost me
<hatseflats> as long as you don't lose yourself
<arcanez> not sure what that means, but apt-get upgrade did it ;)
<frew> for shadow the collector is just the thing that lets auditany use shadow
<vanstyn> so, for the case of the DBIC collector, it drops the datapoints in 3 related tables, and the datapoint names become the column names
<arcanez> frew: 0.4.0-1 now
<frew> wow, brand new
<vanstyn> but other collectors might print STDERR Dumper($context_object)
<frew> vanstyn: what if I want to use a different set of data points at different times?
<frew> vanstyn: (which is why you can just pass more data at ctx_do time for shadow)
<vanstyn> you configure your datapoints on a per-auditor basis
<frew> right
<vanstyn> not sure I understand the question
<frew> like
<frew> imagine we are collecting user and session
<frew> and we have the auditor all set up
<frew> but I also want to log things that happen with logged out sessions
<frew> there is neither a user nor a session
<frew> does it make sense to put user => undef, session => undef into the db?
<vanstyn> well, I would say that is up to you
<frew> I mean does auditany give me the flexibility to leave datapoints out at runtime?
<vanstyn> you could put them into a related table only when present, or have them nulls in one table
<vanstyn> yes
<frew> ok, good
<vanstyn> you don't have to do anything with the datapoints if you don't weant
<frew> so I can also add datapoints at runtime?
<vanstyn> you specify the list of datapoints you want when you create the auditor, so yes
<vanstyn> (if that is your question)
<vanstyn> there are defaults, but you can also specify them
<vanstyn> here: https://github.com/vanstyn/DBIx-Class-AuditAny/wiki/Examples
<vanstyn> look at the second example
<vanstyn> where it defines datapoints to be used
<frew> ok
<frew> but this is what I mean
<frew> you define those one time
<frew> what if I wanted to change them later, for a specific changeset?
<vanstyn> well, again, that is an implementation detail of the collector.... let me back up a bit
<frew> when I say at runtime I mean *after* the audit any has started
<vanstyn> right, well, it depends
* arc has quit (Quit: Leaving.)
<vanstyn> if you want to stop them from being collected, that is one thing... if you want to chose to use the data or not in the collector, that is another
<vanstyn> so which is your concern/question?
<frew> lik
<frew> different from what you said :)
<frew> here's an example
<frew> let's say we are collecting to STDOUT
<frew> that's easy to understand and the data can be anything, easily
<frew> now lets say the data points I collect are just the stacktrace
* arc ([email protected]) has joined #dbix-class
<frew> but for this ONE line, I want to add in some random variable
<frew> I don't want to change the configuration of the auditor for that one thing
<frew> I want to specify: "hey pass this extra datapoint when the context is created"
<frew> "just this once"
<frew> is that doable?
<vanstyn> so you want the value to be calculated differently?
<frew> no
<frew> I want it to be added
<frew> it would never be calculated at all w/o me passing it
<frew> because it's special and unconfigured
<vanstyn> I see, add datapoints later
* frederico ([email protected]) has joined #dbix-class
<frew> right.
<frew> or override them , for example
<vanstyn> I see what you are asking... no, I don't have anything like that, but I approach the question differently such that I would never need to do that... unless the worry is in overhead of collecting a datapoint every time, even if you are only going to use it 1% of the time... it is still collected every time
<frew> no it's jsut that...
<frew> what about stuff that makes no sense in a different context?
<frew> so like, username for example
<frew> you HAVE to pass in the catalyst object or w/e to get that
<vanstyn> username has a context
<vanstyn> I would think its context would be changeset
<frew> sorry, that was a bad example
<frew> go back to a test
<frew> I'm running a test
<vanstyn> remember, the datapoint has a *CodeRef* which can contain logic
<frew> and I have an integer, you know, for my $i (0..10) { # test here)
<frew> right
<frew> but the coderef can't get to $i unless I define the auditor in that specific part of the test
<frew> and I don't want to do that
* fernandinho ([email protected]) has joined #dbix-class
<vanstyn> so its a scope issue ?
<frew> I want to define the auditor for all of my tests when my test db is initialized, when a specific env var (AUDIT_TESTS) is set
<frew> sure, it's a scope issue
* eric ([email protected]) has joined #dbix-class
<frew> but more a flexibility issue that I want you to acknowledge :)
<frew> I think your datapoint thing is cool and good
<frew> but I also think you should consider making it so that users can specify data points *after* the auditor is defined, without modifying the auditors definition
<vanstyn> The API is inspired by catalyst... The idea is the Collector is like a controller, and like controller actions, it is passed a $Context object. So its like your asking "how do I have control over if I use $c->req->address or not?"
<frew> heh, well I wonder if jnap or mst would be happy that you'd base something like this on ideas from catalyst :)
<vanstyn> if you want it, call it, if you don't, dont... I guess I am still missing the point...
<frew> but it's more like the stash
<frew> $c->stash->{foo} # <-- totally free form data
<frew> I want to log a bit of freeform data at runtime without changing the datapoint_configs
<vanstyn> well, no, because by the time you get it the data points have already been collected, just like 'address' has already been collected and is read-only
<frew> right
<frew> exactly
<frew> let me show you a test from shadow :)
<frew> https://github.com/ribasushi/preshadow/blob/master/t/changeset-do.t#L19
<frew> vanstyn: *anything* can go in the params to changeset_do
<vanstyn> ah ha!
<vanstyn> ok, let me explain then....
<frew> I know that you grab the datapoints automatically
<frew> and I say again, that is generally awesome
<frew> but *sometimes* it will be nice to have an interface like this one
<frew> where at the application level the user can add to datapoints w/o more config
<vanstyn> right, so here is the thing, you are asking about datapoints as in *instances* or holders
* frew waits
<vanstyn> which is separate from the value it contains, which changes from changeset/change/column etc from one to the next....
<vanstyn> now, I looked at AuditLog
<frew> (which I've never used btw)
<vanstyn> and there was a concept that I liked, which was the second argument to txn_do
<frew> (and also I am very suspect of all of the auditors out there except for shadow :)
<vanstyn> his idea is you call $txn_do($code,"Some description of change")
<frew> sure
<vanstyn> and then that string is associated with the change
<frew> which is in essence what I'm saying
<frew> except a bit more structured
<vanstyn> so, what I want to do, however, is define the second argument as a hashref that can be used to *populate* the value of changeset datapoints
<frew> which is *exactly* waht shadow does
<frew> (except it's the first arg, but w/e)
<vanstyn> right, yes.... these are API details within code that has to know that it is being audited...
<vanstyn> and that is part of what needs to be figued out... is what API do we expose there
<vanstyn> So, I saw your "changeset_do"
<frew> the reason we didn't override txn_do is because more args to dbic's txn_do already matter
<vanstyn> And I think that should be an option (if I understand its purpose)... to where the ed-user code can call changeset_do instead of txn_do and be able to toggle the behaviour of always hooking into txn_do
<frew> (eg $s->txn_do(sub { ... }, $foo, $bar, $baz), $foo bar and baz get passed to the sub)
<frew> sure
<vanstyn> right, and yes, I think calling a changeset_do is a better api than shoving an argument into a method that doesn't expect it
<frew> ok, just making sure.
<vanstyn> so, right.... this is the part that still needs to be decided.... is the "end-user" APIs
<frew> vanstyn: ok, you have 15 more minutes of my time :)
<frew> sure
<frew> well I really like your examples for initial setup
<vanstyn> as in I bouht a new 15 minutes because you like what you hear, or you only have 15 mins left?
<frew> as for *querying* the collected data
<frew> 15 minutes left :)
<vanstyn> goctha
<frew> it *has* to be diff per collector
<vanstyn> right
<frew> ie if you are collecting to STDERR there is no querying, ever
* vervain has quit (Quit: leaving)
<vanstyn> of course
<frew> ok
<frew> just making sure
<vanstyn> here is why I bring that up
<frew> k
<vanstyn> when I was reading through Shadow, I saw you class level API constructs, which wouldn't fit so well under the object API/logical abstraction
* vervain ([email protected]) has joined #dbix-class
<frew> right
<vanstyn> and also recognizing that even though they are independent, we want to be able to provde a nice API that handles everything the user wants in a nice, clean way
<frew> shadow is DBIC oriented
<frew> and thus is all results and resultsets
<vanstyn> right
<frew> yes I agree in full
<frew> so what we do is create a wrapper api on top
<frew> that makes it such that the user doesn't have to use the results or resultsets directly
<vanstyn> right
<frew> but if the user *wants* to he or she can
<frew> note that many of us have tried and failed to do such a thing
<frew> at work I've decided that generally the best way to do this is to add methods to the schema
* denny has quit (Quit: oooog)
<frew> isntead of having an object that has-a schema
<frew> of course it's not HARD to just have an object that has-a schema
<frew> it just adds a layer that is generally not needed imo
<frew> what we *could* do
<frew> is make a role that defines our end user interface
<frew> and you can apply it directly to a schema if you want
<frew> or apply it to a vanilla object
<vanstyn> yeah, just bumb out one more layer of abstraction up
<vanstyn> *bump
<frew> so does that answer your question?
<vanstyn> which one ? :)
<vanstyn> Can you point me in the direction of how to do the ANON class thing?
<frew> what end-user api should look like
<vanstyn> ah.. well....
<frew> well, with Moo it's Role::Tiny->apply_roles_to_object($obj, @roles)
<vanstyn> That part, imo, doesn't even need to be part of AuditAny, per-se...
<frew> agreed
<vanstyn> it is more a part of the collector implementation
<frew> for Moose it's Class::MOP::Class->create_anon_class
<frew> if you can get your stuff to work 100% under Moo it will ensure that the code isn't too crazy coupled to Moose
<frew> but that's not any kind of requirement
<ilmari> no, that's Class::MOP. for Moose it's Moose::Meta::Class->create_anon_class
<vanstyn> but, again, the reason I bring it up is because of the different paradigm of the API, the current shadow constructs don't *fit* as well... which is why I wanted to make sure it get on the same page with the whole object vs class design approach
<frew> vanstyn: right, see what ilmari said
<ilmari> (yes, Moose::Meta::Class isa Class::MOP::Class)
<frew> vanstyn: sure. I think with shadow making a higher level UI is certainly useable (though not required!)
<frew> er feasible
<frew> noy usable
<vanstyn> Are you saying I can't/shouldn't use Class::MOP? I was also planning on switching to Moo .....
<vanstyn> (for the ANON class stuff)
<frew> just use Moo :)
<frew> apply_roles_to_object will create an anon class based on what the object already use
<vanstyn> then use Moo::Meta::Class ? is there such a thing ?
<frew> no
<frew> sec
<frew> lemme look atyour code again
<frew> vanstyn: right, so instead of adding methods and arounding them and whatnot with Class::MOP
<frew> vanstyn: just make a role with those methods and modifiers
<frew> (a Moo::Role :)
* mmf has quit (Quit: Leaving.)
<frew> and use Role::Tiny to apply said role to the object
<frew> my $meta = Class::MOP::Class->initialize($class); # always scary to see
<vanstyn> I see... add the role on after the fact
<frew> right
<frew> makes the code MUCH more easy for regular people to read too
<vanstyn> is that safe? I seem to recall some gotchas with doing that
<frew> $meta->add_attribute( ... ) # >.<
<frew> has ( ... ) # so nice
<frew> it's the same thing
<frew> what you are doing is just creating a ghetto, non-declarative role
<vanstyn> heh, yeah :)
<frew> so stop doing that :) it's silly
<frew> ok, I gotta go get a food and get back to work
<vanstyn> ok, cool, thanks for the time.... I'll ping you back with more questions later on...
<frew> vanstyn: we can do this again another day if you want, but I'll be mia for most of next week in the snow
<frew> awesome.
<frew> station
<frew> & foodz