Python script using distutils to copy files on a Mac

Question

I have a Python script I have written to copy files to a mounted Windows SMB share on a Mac.

import os
import distutils.core
# Create a local site for the mount to reside
directory = "/Users/username/share"
if not os.path.exists(directory): os.makedirs(directory)
# Mount the Windows smb share
os.system("mount_smbfs //service.account:[email protected]/share ~/share")
# set source and destination
fromDirectory1 = "/Volumes/Macintosh HD/folder1/folder2/folder3"
fromDirectory2 = "/Volumes/Macintosh HD/folder4/folder5/folder6"
fromDirectory3 = "/Volumes/Macintosh HD2/folder1/folder2"
fromDirectory4 = "/Volumes/Macintosh HD2/folder1/folder3/folder4"
toDirectory1 = "/Users/username/share/folder3"
toDirectory2 = "/Users/username/share/folder6"
toDirectory3 = "/Users/username/share/folder2"
toDirectory4 = "/Users/username/share/folder4"
#Do the copying
distutils.dir_util.copy_tree(fromDirectory1, toDirectory1)
distutils.dir_util.copy_tree(fromDirectory2, toDirectory2)
distutils.dir_util.copy_tree(fromDirectory3, toDirectory3)
distutils.dir_util.copy_tree(fromDirectory4, toDirectory4)
#Unmount the share
os.system("umount ~/share")

I understand the script is verbose but I wrote it this way to # out lines to problem solve. Can you suggest a cleaner way to write it?

Any insights gratefully received.

Version 2:

import os
import distutils.core
# Create a local site for the mount to reside
directory = "/Users/username/share"
if not os.path.exists(directory): os.makedirs(directory)
# Mount the Windows smb share
os.system("mount_smbfs //service.account:[email protected]/share ~/share")
# set source and destination
copy_jobs = []
copy_jobs.append({"from": "/Volumes/Macintosh HD/folder1/folder2/folder3", "to": "/Users/username/share/folder3"})
copy_jobs.append({"from": "/Volumes/Macintosh HD/folder4/folder5/folder6", "to": "/Users/username/share/folder6"})
copy_jobs.append({"from": "/Volumes/Macintosh HD2/folder1/folder2", "to": "/Users/username/share/folder2"})
copy_jobs.append({"from": "/Volumes/Macintosh HD2/folder1/folder3/folder4", "to": "/Users/username/share/folder4"})
#Do the copying
for job in copy_jobs:
 distutils.dir_util.copy_tree(job["from"], job["to"])
# Unmount the Windows smb share
os.system("umount ~/share")

• SylvainD: You do not use toDirectory1. Is it normal ?
• John Harris: @Josay typo! Good eyes. Have corrected it. Thanks
• Gareth Rees: It would be better to use shutil.copytree from the standard library instead of the version from distutils.
• John Harris: @GarethRees Thanks for your reply. Never had much luck with shutil - if I import shutil and replace distutils.dir_util.copy_tree with shutil.copytree I get a page of [Errno 22] invalid argument errors.
• Gareth Rees: That probably indicates a bug in shutil.copytree which you should report.

Answer 1

The code looks OK. A few specific improvements can be made.

Readability

import os
import distutils.core

Learn about PEP8, you should have two empty lines between imports and the rest of the code. More generally, you should give more structure to your code to make it more readable. Empty lines are one way to achieve that. For example, one empty line before each comment could be nice.

# Create a local site for the mount to reside
directory = "/Users/username/share"
if not os.path.exists(directory): os.makedirs(directory)

PEP8 comment: prefer indenting ifs to make clear they are ifs.

"Security"

# Mount the Windows smb share
os.system("mount_smbfs //service.account:[email protected]/share ~/share")

Don't store account and password in your source file. You could move them to a configuration file (that would not be tracked if you used git or svn) or retrieve them from the environment (but then they would show up in your shell logs).

Don't Repeat Yourself

# set source and destination
fromDirectory1 = "/Volumes/Macintosh HD/folder1/folder2/folder3"
fromDirectory2 = "/Volumes/Macintosh HD/folder4/folder5/folder6"
fromDirectory3 = "/Volumes/Macintosh HD2/folder1/folder2"
fromDirectory4 = "/Volumes/Macintosh HD2/folder1/folder3/folder4"
toDirectory1 = "/Users/username/share/folder3"
toDirectory2 = "/Users/username/share/folder6"
toDirectory3 = "/Users/username/share/folder2"
toDirectory4 = "/Users/username/share/folder4"

Variables name var1, var2 and so on indicate that you should use a list of pairs or a dictionary!

directories = [
 ("/Volumes/Macintosh HD/folder1/folder2/folder3", "/Users/username/share/folder3"),
 ("/Volumes/Macintosh HD/folder4/folder5/folder6", "/Users/username/share/folder6"),
]
#Do the copying
for from, to in directories:
 distutils.dir_util.copy_tree(from, to)
#Unmount the share
os.system("umount ~/share")

A note about the 80-char limit: in this case, the directories list is more readable if you don't enforce the rule.

Recovering from failures

What happens when mounting works but not copying? You should still try to unmount your folder.

try:
 # mount
 # copy
finally:
 # umount

You can also decide to catch exceptions if you know what to do about them.

• SylvainD: Excellent answer. As an additional comment, depending on how your requirements change in the future, you might want directories to become a list of pairs <from, to> if the same folder is to be copied in different places. The change should be pretty straight forward to perform and the idea is still to use the right data structure instead of duplicating logic.
• Quentin Pradet: Nice spot. I didn't really know which one to choose, I'll switch to a list of pairs. Thanks!
• John Harris: @QuentinPradet Thanks for your insights. I have used a dictionary and revised my code (see version 2 above). Am aware of the security issues but it is on a secure Mac server. Never-the-less I will investigate a config file. I need to put the recovering from failure and perhaps a logging step into the code. Thanks again