Review of code, part 2

[GoogleCodeExporter]

Continuing from yesterday...

*************************************
includes/config.php
*************************************

Why do you have lots of configs here and then more in the database? Since we 
have a database anyway, it seems most user configurable settings should be 
stored there. That way they can be edited from a gui, once it exists.
This file should include:
 - how to connect to database
 - PHP constants

It could be argued that this file is easier as long as a gui doesn't exist, but 
a simple settings table with "setting, value, description" is relatively easy 
to edit with something like phpMyAdmin or MySQL Workbench.

These are specific to your internal environment:
$config['SYSTEM']['xbm_hostname'] = 'bup06-int';
$config['ALERTS']['email'] = 'lmulcahy@marinsoftware.com';

Also this one:
$config['DB']['socket'] = '/mysqldb/tmp/mysql.sock';
...could rather be /var/run/mysqld/mysqld.sock which is the default for deb/rpm 
installations of MySQL. (Assuming you run a dedicated server as backup manager, 
it is not unreasonable to think that it would use the main MySQL instance for 
itself.)

**********************************
service.classes.php / class cronFlusher
**********************************

Tip (for the future, current code works)
I usually just use 
http://fi.php.net/manual/en/function.file-put-contents.php
http://fi.php.net/manual/en/function.file-get-contents.php
...to quickly write a file or read it.

In
            exec("crontab $tmpName", $output, $returnVar);
...use 
crontab -u $config['SYSTEM']['user'] $tmpName

This will prevent anyone from accidentally installing this as their own 
crontab. In fact, it allows me to do sudo xbm_flush from wherever, and it will 
use crontab of the xbm user.

Original issue reported on code.google.com by henrik.i...@gmail.com on 25 Mar 2011 at 2:04

[GoogleCodeExporter]

Hi Henrik - thanks for the feedback!

I updated the cron flusher to use the -u parameter if "whoami" does not match 
the user in $config['SYSTEM']['user'] 

I had to do this because the crontab command will not let you use the -u option 
unless you are a "privileged user" -- this even applies if you are, for 
example, currently logged in as "xbm" user and attempt to run a command like

crontab -u xbm myCronFile

We will hit an error if someone is running as a user that is not privileged and 
also not the xtrabackup manager configured user, but this should be OK.

As for the config,php feedback - will respond in due course :)

Original comment by lachlan....@gmail.com on 25 Mar 2011 at 7:41

[GoogleCodeExporter]

Original comment by lachlan....@gmail.com on 25 Mar 2011 at 10:43

[GoogleCodeExporter]

Closing this out as crontab fix is now done and we have a new issue for moving 
config.php into the DB.

http://code.google.com/p/xtrabackup-manager/issues/detail?id=18

Original comment by lachlan....@gmail.com on 1 Apr 2011 at 9:46

• Changed state: Resolved