HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python script using distutils to copy files on a Mac

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptdistutilsfilesusingpythonmaccopy

Problem

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:password@server.domain.com/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:password@server.domain.com/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/sh

Solution

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:password@server.domain.com/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.

Code Snippets

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:password@server.domain.com/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"
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")

Context

StackExchange Code Review Q#41405, answer score: 4

Revisions (0)

No revisions yet.