#1
  1. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Apr 2013
    Posts
    3
    Rep Power
    0

    Question Review of my script


    Hello everyone, I'm new in perl and I'm a student in ICT. I need to write some scripts for my internship. Over 3 weeks there is a jury who judge over the internship and they will see my scripts. But it could be that some of the people are experts in perl or scripting. I would like to ask if some of you will check my script and give some suggestions and feedback. The script is to make a dump of all the databases from remote. The scripts works fine normally. Many thanks for the suggestions!

    Code:
    #!/usr/bin/perl
    # (c) Odeurs Dieter
    # Script to back-up the databases
    
    use strict;
    use warnings;
    use Getopt::Long;
    use Net::Ping;
    use Time::Local;
    
    # Variables
    my ($host, $engine, $user , $password, $port, $cmd);
    my ($ping, $all,$timestamp, $filename, $help, $error, $result);
    my @result;
    my @args = ();
    
    #-----------------------------------------------
    # Show usage
    #-----------------------------------------------
    sub showUsage {
    	print <<EOF;
    
    ------------------------------------------
    -- Usage for the database backup script --
    ------------------------------------------
    
    --host
    	  Specify the host IP
    
    --engine
    	  Specify the database engine. This could be postgres or mysql.
    
    --password
    	  Give the password needed to connect with the database server
    
    --user
    	  Specify user to connect to the database server.
    	  The user must have permissions to connect to the database(s)
    
    --port
    	  If another port than the standard must be used to connect you could
    	  specify this with the --port option
    EOF
    exit;
    }
    
    # Read Options
    GetOptions(
    	"host=s"	=> \$host,
    	"engine=s"	=> \$engine,
    	"password=s"	=> \$password,
    	"user=s" 	=> \$user,
    	"port=s"    => \$port,
    	"all"		=> \$all,
    	"help"		=> \$help
    );
    
    if ($help){
    	showUsage();
    }
    
    #Creating timestamp 
    my ($sec, $min, $hour, $day, $month, $year, $wday, $yday, $isdst) = gmtime(time);
    $timestamp = sprintf("%4d-%02d-%02d_%02d-%02d", $year + 1900, $month + 1, $day, $hour, $min);
    
    #------Check options-----
    # Check if host is defined and alive
    unless (defined $host) {
        warn "\nNo host specified\n\n";
        showUsage ();
    }
    print "\nChecking if $host is alive.. \n";
    Net::Ping->new ()->ping ($host)
        or die "$host is unreachable\n";
    
    print "$host is alive\n\n";
    
    #If a password is set we need to set PGPASSWORD environment variable
    if ($password && $password ne ''){
    	if ($engine eq "postgres" ){
    		$ENV{'PGPASSWORD'} = $password;
    	} else {
    		push(@args, qq{--password=$password});
    	}
    }
    
    if ($host && $host ne ''){
    	push(@args, qq{--host $host});
    } 
    
    if ($port && $port ne ''){
    	push(@args, qq{--port $port});
    } 
    
    if ($user && $user ne ''){
    	if ($engine eq "postgres" ){
    		push(@args, qq{--username $user});
    	} else {
    		push(@args, qq{--user $user});
    	}
    } 
    	
    #Check if database engine is defined and valid
    if (defined $engine) {
    	# When a engine is defined the trigt sub will be called 
    	if ($engine eq "postgres" ){
    		postgres();
    	} elsif ($engine eq "mysql" ){
    		mysql();
    	} else {
    		print "Unknown database engine\n";
    		showUsage();
    	}
    } else {
    	print "Please specify the database engine postgres|mysql\n";
    	showUsage();
    }
    
    #------------------------------------------------
    # Postgres
    #------------------------------------------------
    sub postgres{
    	if ($all) {
    		$filename = "/home/dieter/dumps/postgres/" . "Fulldump" . "_" . $timestamp . ".sql";
    		print "Taking dump of all databases...\n";
    		$cmd = "pg_dumpall @args > $filename";
    		@result = `$cmd`; 
    		if ($? !=0) {
    			warn "Done but with errors\n\n";
    		} else {
    			print "Dump saved in $filename\n\n";
    		}
    	} else {
    		#Find al the databases on the specified host
    		print "Checking databases.. \n";
    		$cmd = "psql -At @args -c 'SELECT datname FROM pg_database'";
    				
    		#Print the non-standard databases and store them in an array
    		my @databases;
    		@result = `$cmd`;
    		if ($? !=0) {
    			warn "Some errors occured..\n\n"; 
    			exit;
    		}
    		print "Databases to back-up: \n";
    		foreach $result (@result) {
    			chomp $result;
    			if ($result !~ "template" && $result !~ "postgres"){
    				print "   - " . $result . "\n";
    				push(@databases, $result);
    			}
    		}	
    		
    		#Back-up each database
    		foreach my $database (@databases){
    			#Generate a filename for each database
    			$filename = "/home/dieter/dumps/postgres/" . $database . "_" . $timestamp . ".sql";
    			$cmd = "pg_dump -C @args $database > $filename";				
    			print $cmd . "\n" ;
    			@result = `$cmd`;
    			if ($? !=0) {
    				warn "Done but with errors\n\n"; 
    				exit;
    			}else {
    				print "DONE \n";
    			}
    		}
    	}
    	
    }
    
    #-----------------------------------------------
    # MySQL
    #-----------------------------------------------
    sub mysql{
    	#-------One database in one file------
    	if ($all) {
    	#-------All database in one file------
    	$filename = "/home/dieter/dumps/mysql/" . "Fulldump_" . $timestamp . ".sql";
    		print "Taking dump of all databases...\n";
    		$cmd = "mysqldump @args --all-databases > $filename";
    		@result = `$cmd`;
    		if ($? !=0) {
    			warn "Done but with errors\n\n"; 
    			exit;
    		}else {
    			print "Full dump saved in $filename\n";
    		}	
    	} else {
    			#list databases and exclude the default
    		my @databases;
    		print "Checking databases..\n";
    		$cmd = "mysql @args -Bse 'show databases'";
    		@result = `$cmd`;
    		if ($? !=0) {
    			warn "Some errors occured..\n\n"; 
    			exit;
    		}
    		print "Databases to back-up: \n";
    		foreach $result (@result) {
    			if ($result !~ "information_schema" && $result !~ "mysql"){
    				print "   -  $result \n";
    				push (@databases, $result);
    			}
    		}
    		
    		push(@args, qq{--single-transaction});
    		#back-up of the selected databases
    		foreach my $database (@databases){
    			chomp $database;
    			$filename = "/home/dieter/dumps/mysql/" . $database . "_" . $timestamp . ".sql";
    			print "Taking dump of $database ...\n";
    			$cmd = "mysqldump @args $database > $filename";
    			@result = `$cmd`;
    			if ($? !=0) {
    				warn "Done but with errors\n\n"; 
    				exit;
    			}else {
    				print "Dump saved in $filename\n";
    			}
    		}
    	}
    	
    	print "Dump completed!\n\n";
    }
    exit;
  2. #2
  3. No Profile Picture
    Contributing User
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Apr 2009
    Posts
    1,940
    Rep Power
    1225
    I see that you made one adjustment (i.e., the heredoc) that Bill suggested on your cross post at perguru. It just happens to be the one that I'd do differently.

    Instead of using a heredoc, I'd use POD documentation. That way you could have better documentation and better handling of displaying the usage statement if required args are missing.

    The Getopt::Long module has a short example on using Pod::Usage.
    http://search.cpan.org/~jv/Getopt-Lo...and_help_texts

    More detailed info can be found in the Pod::Usage module documentation.

    You're doing too much testing for missing args. Or, should I say that your testing is too verbose. Here's how you can do that testing with a single simple statement.
    Code:
    pod2usage(2) unless ($host and $engine and $user and $password);
    That could be expanded as needed.

    The very next test should be on the $engine value and execute the proper sub based on that value. I'd use a dispatch table to handle this step.

    Using ping to test if a host's echo port is accessible is "ok", but insufficient in this case. Firewall rules could be blocking icmp traffic, or the service/port that you really need to connect to, such as 3306 for mysql, could be closed. So, if you want to do this kind of test, you should try to connect using tcp to the port that the database is listening on.

    Var names should describe the data that they hold. Most of yours are ok in this respect, but a couple of them should be adjusted. $all doesn't tell me much. All of what? It might be better to name it something like $all_databases.

    EDIT:
    A dispatch table is a hash of code references. The example that space_monk gave on your perlmonks cross post is a dispatch table.
    Last edited by FishMonger; May 15th, 2013 at 09:32 AM.

IMN logo majestic logo threadwatch logo seochat tools logo