----- Original Message -----
From: "Matt Wagner" <matt.wagner(a)redhat.com>
To: "Tzu-Mainn Chen" <tzumainn(a)redhat.com>
Cc: aeolus-devel(a)lists.fedorahosted.org
Sent: Tuesday, April 3, 2012 11:53:22 AM
Subject: Re: RFC Feature Planning: Deployment Activity History
On Tue, Apr 03, 2012 at 12:05:48AM -0400, Tzu-Mainn Chen wrote:
> Thanks for the comments! My comments in-line:
>
> ----- Original Message -----
> > From: "Matt Wagner" <matt.wagner(a)redhat.com>
> > To: "Tzu-Mainn Chen" <tzumainn(a)redhat.com>
> > Cc: aeolus-devel(a)lists.fedorahosted.org
> > Sent: Monday, April 2, 2012 9:49:02 AM
> > Subject: Re: RFC Feature Planning: Deployment Activity History
> >
> > On Fri, Mar 30, 2012 at 11:59:11AM -0400, Tzu-Mainn Chen wrote:
> > >
https://www.aeolusproject.org/redmine/projects/aeolus/wiki/Allow_Conducto...
> >
> > So I wrote the below email and was about to send it, when I
> > realized
> > that I'd focused on the technical implementation details without
> > ever
> > fully understanding the overall use case. But it's not abundantly
> > clear
> > to me what that entails. What information, exactly, are we
> > intending
> > to
> > keep? I wonder if we should look at stashing core information in
> > the
> > Events table?
> >
>
> To be honest, I assumed that the scope of the task was simply to
> create a way to store events, and then worry about (potentially)
> adding events later. I'd hope that the implementation certainly
> wouldn't preclude that possibility, and expandability is
> definitely something worth keeping in mind.
So I certainly don't mean to point the finger at you here, because I
think you're right about this task only referring to the technical
changes. But I will note that this is the second major "Events" task
to
be entered without any user stories about what history we want to
save,
or why. Normally I'm not too big of a stickler about user stories,
but
in the case of storing events, I think it's really pretty important
to
know *what* we want to save. I suspect you've done the best that can
be
done without more background, but I think we could make more progress
if
we knew what it is that we're building.
No, that's a fair point. I'll update the doc and seek clarification.
> > > The document talks about two possible approaches to keeping
> > > deployment activity; one is to use an archive table, and
> > > another
> > > is to use a soft_delete flag. I'd be interested in hearing
> > > people's thoughts on both.
> >
> > I was going to mention that I'm still fond of acts_as_paranoid,
> > but I
> > just saw your mention on the wiki page that Tomas sent a patch
> > adding
> > archivist six months ago. I'm all for just using that in this
> > case. I
> > think we're likely to need to stitch together associations anyway
> > when
> > we're dealing with deleted objects.
> >
> > Though, FWIW, using soft delete wouldn't necessarily require any
> > major
> > changes. acts_as_paranoid would override the finder methods of AR
> > to
> > add
> > a "WHERE deleted_at IS NULL" to all queries unless you ran
> > "find_with_deleted". If we did this ourselves, we could probably
> > just
> > use a default scope and a with_deleted method to override it.
> >
>
> I personally prefer the soft delete approach, because my experience
> with playing with associations is not good; it's possible a bad
> association took my lunch money in grade school.
has_no :lunch_money ?
I think this is a good point, though. We have some code to check if
things have the "last reference" to something on has_many
relationships,
which would probably result in things being deleted if we switched to
archive tables.
Although, actually, we're still going to have to deal with this. Take
deployments, which can't be deleted if they still have instances.
(I'm
going from memory here, so if that's not true, pretend it is.) Now if
you "delete" all the instances, that code needs to be changed to look
for non-deleted instances only, and to do a soft-delete on the
deployment in that case. So I'm left wondering if maybe we're going
to
need to shape some code to suit our needs whether we use archive
tables
or soft deletes.
(Note, though, that I'm personally in the "soft delete" camp myself,
I'm
just thinking of ways to not reinvent the wheel after Tomas already
sent
out an archive table patch a few months ago.)
> > > Oh, and the document also talks about potentially using
> > > dbomatic
> > > for periodic history purges. I know there's been some
> > > discussion
> > > around dbomatic, so if someone had an opinion about that as
> > > well.
> > > . .
> > >
> >
> > I think I talked to you on IRC about this and didn't note this
> > objection, so I apologize for suddenly raising it. ;) But it
> > occurs
> > to
> > me now that cleaning up old history is something that we want to
> > do
> > perhaps once a month, not every couple of minutes, so maybe we
> > should
> > just use cron. What if we added a rake task / little script that
> > did
> > this, using a configurable time period? Then we could ship with a
> > reasonable default (do a monthly cleanup of entries older than 6
> > months?), but allow administrators to change it by just editing
> > crontab.
> > (I'm also not entirely convinced that deleting archived entries
> > by
> > default is a good thing... Some users might want to preserve
> > history
> > indefinitely. This way they could at least disable it easily.)
>
> There's a separate task (specified in the document) that specifies
> that the administrator should be able to configure history purges.
> I should add details to that section to specify what I think that
> means. In any case, I disagree that cleaning up old history is a
> once-a-month task, but the point about cron still stands. What I
> *do* want to avoid is the sense that Conductor is using multiple
> schedulers for multiple tasks when one scheduler could do; I don't
> think that's a pleasant user experience. Ideally, though, the
> action and the scheduler should be reasonably discrete. I'll make
> a note of the issue in the documentation.
Oh, hmm -- does a "configurable policy" mean that you should be able
to
change it through the web interface? Because that potentially means
that
we can't "just use cron" here. Though I can see how being able to
change
this on the fly would actually be a bad thing -- someone accidentally
changes the archive period to 1 hour, saves, loses almost all
history,
and then changes it back to 90 days and everyone is left wondering
where the history went. (That said, I'm really not a big fan of
disabling features because someone could misuse them. But it makes me
wonder if maybe the interval should just be set in a config file, not
on-the-fly.)
This might be a wider philosophical point, is the current standard to use a config file?
-- Matt