[Xtrabackup Manager] discussion

5 views
Skip to first unread message

Henrik Ingo

unread,
Mar 28, 2011, 11:03:18 AM3/28/11
to Lachlan Mulcahy, perco...@googlegroups.com
Hi Lachlan

So the final ingredient in running a successful open source project:
avoid private emails, push everything to a mailing list or IRC.

I'll make a bold assumption that the Percona guys don't mind if we
discuss Xtrabackup Manager on this list that is also home for
Xtrabackup development.

Here continues random review'ish notes from today's experiments:

**
Commit message for r16:
http://code.google.com/p/xtrabackup-manager/source/detail?r=16&path=/trunk/includes/config.php
Also disabled PHP warnings in config.php by default for cleaner output.

This bit me a little. By default Ubuntu comes without php-mysql
installed, so anything in xbm fails due to missing the mysqli library.
The first time this fails is just subclassing the dbConnection from
mysqli and since no object or even the class itself doesn't exist at
this point, of course your error handling doesn't do anything either.
So the result is that the script silently fails and does nothing.

I'd personally let the default be to PHP print the errors as long as
heavy development goes on.

**

You are being very careful with validating input for correctness, etc.
Unfortunately this makes the code verbose and hard to read. Going
forward it would be great if you adopted the use of exceptions for
signaling errors, so that one needn't check the return value of every
single function call.

http://php.net/manual/en/language.exceptions.php

With exceptions you can run through a large block of code without
worrying about errors, then add a catch() statement at the end. This
makes code much more readable. (And hey, this is PHP. Many of us
writes PHP code without worrying about errors altogether :-)

The good thing about exceptions is that you can also throw them from a
object constructor. For instance, in backupSnapshot.class.php this
check:

if(!is_numeric($this->id)) {
$this->error = 'backupSnapshot->setSnapshotTime:
'."Error: The ID for this object is not an integer.";
return false;
}

Should be done in the constructor. (But since you cannot return false
from a constructor, I understand why you didn't. Use exceptions!)


Ok, done for today.

henrik
--
henri...@avoinelama.fi
+358-40-5697354 skype: henrik.ingo irc: hingo
www.openlife.cc

My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559

Baron Schwartz

unread,
Mar 28, 2011, 11:12:58 AM3/28/11
to perco...@googlegroups.com
Henrik,

> So the final ingredient in running a successful open source project:
> avoid private emails, push everything to a mailing list or IRC.
>
> I'll make a bold assumption that the Percona guys don't mind if we
> discuss Xtrabackup Manager on this list that is also home for
> Xtrabackup development.

Amen. I've become something of a curmudgeon about getting off-list
emails. Unless it's got *private* data, the question belongs in
*public* for everyone's benefit, and so that one person doesn't become
the bottleneck for answering questions. And if it has private data,
then either the sender should be taking some extra time to generate a
reproducible test case without private data, or paying for support.

I think this mailing list is fine, although if Lachlan would prefer to
set up a separate one that is fine too.

Baron

Reply all
Reply to author
Forward
0 new messages