aag / apple_trailer_downloader

A script to download HD trailers from the Apple Trailers website.
GNU General Public License v3.0
23 stars 5 forks source link

File containing download files was wiped! #6

Closed forthrin closed 8 years ago

forthrin commented 8 years ago

A serious issue here:

After downloading the trailer for "The Summer of Sangailé", the script quit and afterwards the download_list.txt files was 0 bytes! Thus every trailer will be downloaded again.

Suspect the problem came from the "é" in the name. For future safety, consider writing the file list to a temporary file and replace it after doing a simple sanity check (eg. file size equal to or larger than original.)

~$ python ~/bin/trailer.py 
Checking for The Summer of Sangailé
downloading http://movietrailers.apple.com/movies/independent/thesummerofsangaile/summerofsangaile-tlr1_h1080p.mov
Saving file to /Users/***/Movies/Trailers/The Summer of Sangailé.Trailer.1080p.mov
Traceback (most recent call last):
  File "/Users/***/bin/trailer.py", line 333, in <module>
    config['video_types']
  File "/Users/***/bin/trailer.py", line 219, in downloadTrailersFromPage
    recordDownloadedFile(trailerFileName, dlListPath)
  File "/Users/***/bin/trailer.py", line 189, in recordDownloadedFile
    writeDownloadedFiles(fileList, dlListPath)
  File "/Users/***/bin/trailer.py", line 182, in writeDownloadedFiles
    f.writelines(newList)
TypeError: writelines() argument must be a sequence of strings

Truncated file :(

~/Movies/Trailers$ ls -l
-rw-r--r--   1 user  staff          0 Oct 30 16:11 download_list.txt

PS! While we're on the subject, also suggest to keep the config/list files in the $HOME directory (see below) rather than the application and download directory where they are now.

$HOME/.trailer.cfg
$HOME/.trailer.lst
forthrin commented 8 years ago

Any status on this? I'm effectively unable to stay updated on trailers because of this issue.

aag commented 8 years ago

After downloading the trailer for "The Summer of Sangailé", the script quit and afterwards the download_list.txt files was 0 bytes! Thus every trailer will be downloaded again.

This is fixed now. The download list now gets saved with UTF-8 encoding, so it should be able to handle any filenames it gets from the trailers site. Please update the script and try it again.

While we're on the subject, also suggest to keep the config/list files in the $HOME directory (see below) rather than the application and download directory where they are now.

I prefer the default locations for the config and list files, so I'm going to leave them where they are. However, you can now set the location of the config file with the -c command-line option and the location of the list file either with the -l command-line option or the list_file option in the config file. So, if you want to use the locations you mentioned, you could run the script like this:

$ python download_trailers.py -c ~/.trailer.cfg -l ~/.trailer.lst

For future safety, consider writing the file list to a temporary file and replace it after doing a simple sanity check (eg. file size equal to or larger than original.)

I'm still considering different options for the best way to write the file reliably and quickly, so I haven't implemented this yet.

Please try out the newest version of the script and let me know if you run into any issues.

forthrin commented 8 years ago

Seems to work now!

About the cfg/lst files, I thought $HOME was a cleaner and more UNIX-y approach. Adding command line options makes the tool more cumbersome. It's nice to be able to type trailer.py and off you go.

forthrin commented 8 years ago

PS! One annoying thing about having download_list.txtin the trailer directory, is that it appears in the middle of all the trailers (see below), so if you want to select and open all of them to view them successively in your video player, you always have to unselect that one file. It would be nice to put it somewhere else as default, or rename to .download.txt to get it out of the way or something.

Asthma.Trailer.1080p.mov Daddys Home.Trailer.1080p.mov download_list.txt Hail, Caesar.Trailer.1080p.mov ...

aag commented 8 years ago

I've updated the script to look for a configuration file in $HOME/.trailers.cfg if it doesn't find a settings.cfg in the script directory.

So, if you want to have both the config and list files in your home directory, you can move your settings.cfg to $HOME/.trailers.cfg and add this line:

list_file = ~/.trailers.lst

Then the script will use those two file locations, even when it's run without command-line arguments.

Is that sufficient for what you want to do?

forthrin commented 8 years ago

Good decision! And it seems to work nicely. A minor detail: I noticed that download_dir does not seem to accept ~ in the path, while list_file does.

aag commented 8 years ago

I've fixed the ~ problem in download_dir paths.