Beefy Boxes and Bandwidth Generously Provided by pair Networks
"be consistent"
 
PerlMonks  

Re: Zip script not running on cron

by Paladin (Vicar)
on Jul 18, 2019 at 15:27 UTC ( #11102979=note: print w/replies, xml ) Need Help??


in reply to Zip script not running on cron

The line here:
glob("*-".$now.".csv")
is globing in the current directory, which I imagine is /path/to/directory/ when you run the script manually, but I imagine is not the CWD when run from cron.

As a side note, when a script works fine from a shell, but not cron, the issue is almost always either CWD, or some ENV variable.

Replies are listed 'Best First'.
Re^2: Zip script not running on cron
by davido (Cardinal) on Jul 18, 2019 at 15:43 UTC

    ...or permissions for the real/effective user running the cron.


    Dave

Re^2: Zip script not running on cron
by PerlMonger79 (Novice) on Jul 18, 2019 at 16:09 UTC

    Thank you for your time and support. I'm fairly new to perl and I'm really starting to like coding in perl but I've never used CWD nor ENV. I have read from both perldoc and perlmaven but I'm not understanding how to fit it in my script. Is this right?

    #!/usr/bin/perl -w use strict; use warnings; use Cwd qw(cwd); use POSIX qw/strftime/; use IO::Compress::Zip qw(:all); # Timestamp my $now = strftime("%d%b%Y_%H00", localtime(time - 60*60)); # Where zip file is to be stored my $arc = "/path/to/archive/"; # Where csv files are found. chdir "/path/to/directory"; # Zipping files found and storing into archive zip [ glob("*-".$now.".csv") ] => $arc.$now.".zip" or die "Cannot create zip file: $ZipError" ; # Deleting csv files unlink glob("*-".$now.".csv");

      Welcome to Perl and the Monastery, PerlMonger79!

      My experience with cron has been that the safest thing to do is always use absolute pathnames for everything in the crontab and everything that is run from the crontab, which is basically also what Paladin meant with the CWD issue. In this case, I think it's unlikely that anything is affected by %ENV variables, but it's important to keep it in mind.

      In this case, the first thing that jumps out at me is that you're not checking the chdir for errors, as in e.g. chdir "/path/to/directory" or die "chdir: $!";

      Although not critical, there are several other suggestions I'd make:

      • You don't need both use warnings and perl -w on the shebang, you can drop the latter - see also What's wrong with -w and $^W.
      • You don't need the use Cwd qw(cwd); since you're not using that function, it would only become necessary if you wanted to change back to the previous directory (although I like File::pushd for that).
      • Consider moving the two paths out of the body of the script and into variables/constants defined at the top of the script, so they're easier to change globally and could later be turned into subroutine arguments, command-line arguments, or configuration file options. This would become especially important if the script starts getting longer.
      • POSIX's strftime is ok, but nowadays I'd recommend the core module Time::Piece, or for more complex tasks (e.g. anything involving time zones or complex math) DateTime.
      • Regarding glob, be aware of all of the caveats described in To glob or not to glob. In this case, in a short script where you use it in list context, and the pattern is a known pattern without whitespace, it's probably ok. Two alternatives might be Path::Class, or the more low-level opendir+readdir (for the latter, File::Spec's no_upwards is useful).
      • In any case, I'd store the list of filenames in an array instead of building the list twice.
      • If you're certain you'll always be running this script on a *NIX OS, it's ok, but personally I prefer to use File::Spec for filename manipulation anyway (or Path::Class, with a nicer API).
      • You can also check unlink for errors; you might not need to die, you could just warn if there are problems, that's up to you.
      • A couple more diagnostics in unexpected cases are helpful.
      • As I mentioned above, consider providing the absolute path of the perl binary in the crontab entry as well as the absolute path of the script, i.e. /usr/bin/perl /path/to/script.pl. (Update: Doing this, i.e. calling the interpreter and telling it which script to run, also avoids the issue that may arise if you have multiple perl binaries installed and your script uses the shebang #!/usr/bin/env perl and env picks the wrong perl.)
      • If you need more advanced ZIP file handling, consider Archive::Zip.

      Here's a script reworked with several of those suggestions:

      #!/usr/bin/perl use warnings; use strict; use Time::Piece; use File::Spec::Functions qw/catfile/; use IO::Compress::Zip qw/zip $ZipError/; # Timestamp my $now = localtime; $now -= 60*60; $now = $now->strftime("%d%b%Y_%H00"); # Where zip file is to be stored my $arc = catfile("/path/to/archive","$now.zip"); die "Error: $arc already exists, refusing to overwrite" if -e $arc; # Where csv files are found. chdir "/path/to/directory" or die "chdir: $!"; # Zipping files found and storing into archive my @files = glob("*-$now.csv"); warn "Warning: No files to ZIP found\n" unless @files; zip \@files => $arc or die "Cannot create zip file: $ZipError" ; # Deleting csv files unlink(@files)==@files or warn "Failed to unlink some files: $!";

      Update: A few grammar improvements. Apparently I work for the Department of Redundancy Department, apparently.

        Thank you for the warm welcome and thank you very much for the suggestions and especially your version of the script. It worked perfectly ... I appreciate it very much. :)

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://11102979]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others romping around the Monastery: (7)
As of 2019-10-22 13:27 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Notices?