2
\$\begingroup\$

After realizing that I was spending a lot of time getting the latest version of Chromium, I figured I would throw together a quick script. Over time, I wanted to make the script more efficient, cleaner, and pythonic. Here is what I came up with:

from zipfile import ZipFile
from os import rename, remove, path
from shutil import rmtree, copytree
from httplib2 import Http
from sys import argv, exit
from easygui import indexbox


h = Http('.cache')
saveDir = 'C:\\Program Files\\Chromium\\'

def getLatestVersion():
    verSite = ('http://build.chromium.org/f/chromium/snapshots/' +
            'Win/LATEST')
    re, ver = h.request(verSite)
    ver = str(ver, encoding='utf8')
    return ver


def delCurrent():
    try:
        rmtree(path.join(saveDir, 'Current'))
    except Exception:
        print('Chromium could not be removed.')


def delBackup():
    try:
        rmtree(path.join(saveDir, 'Backup'))
    except Exception:
        print('Backup could not be removed.')


def downloadChromium(ver):
    site = ('http://build.chromium.org/buildbot/snapshots/Win/'
            + ver + '/chrome-win32.zip')
    re, chrome = h.request(site)
    file = open(path.join(saveDir, 'latest.zip'), 'wb')
    file.write(chrome)
    file.close()


def unzip():
    zip = ZipFile(saveDir + 'latest.zip', 'r')
    zip.extractall(saveDir)
    rename(path.join(saveDir, 'chrome-win32'), path.join(saveDir, 'Current'))
    zip.close()
    remove(path.join(saveDir, 'latest.zip'))


def revert():
    delCurrent()
    copytree(path.join(saveDir, 'Backup'), path.join(saveDir, 'Current'))


def backup():
    delBackup()
    copytree(path.join(saveDir, 'Current'), path.join(saveDir, 'Backup'))


def gui():
    ver = getLatestVersion()
    choices = ['Download version %s' % ver, 'Backup', 'Revert', 'Exit']
    choice = indexbox('What do you want to do?',
            'Chromium Downloader', choices)
    if choice == 0:
        delCurrent()
        downloadChromium(ver)
        unzip()
    elif choice == 1:
        backup()
    elif choice == 2:
        revert()
    elif choice == 3:
        exit()
    gui()


def usage():
    print('-h         Display help text\n' +
          '-g         Launches the GUI Program\n' +
          '-v         Only gets the version\n' +
          '-r         Reverts to last backup\n' +
          '-b         Saves a new backup\n' +
          '-o         Specify version to download\n')


def main():
    if '-g' in argv:
        gui()
        exit()
    elif '-h' in argv:
        usage()
        exit(2)
    elif '-r' in argv:
        revert()
        exit()

    if '-o' in argv:
        ver = argv.index('-o') + 1
    else:
        ver = getLatestVersion()
        print('Latest Version: ', ver)
    if '-v' in argv:
        exit()

    delCurrent()
    downloadChromium(ver)
    unzip()

    if '-b' in argv:
        backup()

if __name__ == "__main__":
    main()

Are there any major things that should be changed to make this code better?

\$\endgroup\$
2
\$\begingroup\$

I assume you are aware of Chrome Canary, which I gather is able to use binary deltas? I don't think it's pure Chromium though.

Your code is pretty clean. You could be using argparse for parsing the command line, currently you are ignoring flags that don't match. You could wrap the exit call around main, like this: exit(main()), and use return / return 2 statements instead. You could replace "C:\\Program Files" with %PROGRAMFILES%, or pick up some defaults from the registry, but that's not necessary unless you plan for a larger audience. You could also reduce the window Chromium is unavailable by unzipping somewhere else, then moving the directories, then clearing the previous one (but I don't think you can make it atomic without something like symlinks or hardlinks).

\$\endgroup\$
1
\$\begingroup\$
from zipfile import ZipFile
from os import rename, remove, path
from shutil import rmtree, copytree
from httplib2 import Http
from sys import argv, exit
from easygui import indexbox

Its a personal preference thing, but I suggest not using from x import y, unless you use the imported name repeatedly. For example, you only use zipfile once. I'd rather import zipfile and then use zipfile.ZipFile for that one occourance. I think it makes the code a bit cleaner.

h = Http('.cache')

h isn't a very descriptive name. I recommend picking something that gives a better idea what is going on.

saveDir = 'C:\\Program Files\\Chromium\\'

The python style guide recommends ALL_CAPS for global constants

def getLatestVersion():

The python style guide recommends lowercase_with_underscores for global function names

    verSite = ('http://build.chromium.org/f/chromium/snapshots/' +
            'Win/LATEST')

If the URL is long enough to require breaking across multiple lines, its probably long enough to move into a global constant. Also, why are you concatenation two constant strings?

    re, ver = h.request(verSite)

I'm left clueless as to what re and ver are.

    ver = str(ver, encoding='utf8')

Is ver a string? Why not use the decode method?

    return ver

Don't assign the value into a local variable and then return it. Just return it.

def delCurrent():
    try:
        rmtree(path.join(saveDir, 'Current'))
    except Exception:
        print('Chromium could not be removed.')

Firstly, you are catching all exceptions. This is especially bad in python since everything can be an exception. Its better to catch only those exceptions you actually expect. Secondly, you try to continue on after a failure. If you can't remove the current version, you should probably abort and not try to blund on. Thirdly, you don't display any information about what happened. The exception may contain useful information about why it couldn't delete the files.

def delBackup():
    try:
        rmtree(path.join(saveDir, 'Backup'))
    except Exception:
        print('Backup could not be removed.')


def downloadChromium(ver):
    site = ('http://build.chromium.org/buildbot/snapshots/Win/'
            + ver + '/chrome-win32.zip')

I string use of string formatting would make this nicer.

    re, chrome = h.request(site)
    file = open(path.join(saveDir, 'latest.zip'), 'wb')
    file.write(chrome)
    file.close()

Does the api you are using support downloading to a file? If the zip file is very big, downloading it into a string variable may not be a good plan.

def unzip():
    zip = ZipFile(saveDir + 'latest.zip', 'r')
    zip.extractall(saveDir)
    rename(path.join(saveDir, 'chrome-win32'), path.join(saveDir, 'Current'))
    zip.close()
    remove(path.join(saveDir, 'latest.zip'))


def revert():
    delCurrent()
    copytree(path.join(saveDir, 'Backup'), path.join(saveDir, 'Current'))


def backup():
    delBackup()
    copytree(path.join(saveDir, 'Current'), path.join(saveDir, 'Backup'))


def gui():
    ver = getLatestVersion()
    choices = ['Download version %s' % ver, 'Backup', 'Revert', 'Exit']
    choice = indexbox('What do you want to do?',
            'Chromium Downloader', choices)
    if choice == 0:
        delCurrent()
        downloadChromium(ver)
        unzip()

I recommend moving this in a function to match the other options.

    elif choice == 1:
        backup()
    elif choice == 2:
        revert()
    elif choice == 3:
        exit()

I'd put all these functions in a list and then index them.

    gui()

Don't use recursion here, use a while True loop.

def usage():
    print('-h         Display help text\n' +
          '-g         Launches the GUI Program\n' +
          '-v         Only gets the version\n' +
          '-r         Reverts to last backup\n' +
          '-b         Saves a new backup\n' +
          '-o         Specify version to download\n')


def main():

I'd suggest passing in argv to main. That way another piece of code could drive this code by passing arguments to main.

    if '-g' in argv:
        gui()
        exit()
    elif '-h' in argv:
        usage()
        exit(2)
    elif '-r' in argv:
        revert()
        exit()

    if '-o' in argv:
        ver = argv.index('-o') + 1
    else:
        ver = getLatestVersion()
        print('Latest Version: ', ver)
    if '-v' in argv:
        exit()

Using exit to avoid following anymore logic is considered bad form. Use an else block.

    delCurrent()
    downloadChromium(ver)
    unzip()

    if '-b' in argv:
        backup()

if __name__ == "__main__":
    main()
\$\endgroup\$
0
\$\begingroup\$

Some notes:

  • In Python 2, file is a built-in function, so avoid using the name for anything else. More serious is zip, since it's still a built-in function in Python 3.

  • Take advantage of the with statement:

    # old
    file = open(path.join(saveDir, 'latest.zip'), 'wb')
    file.write(chrome)
    file.close()
    
    # new
    with open(path.join(saveDir, 'latest.zip'), 'wb') as f:
        f.write(chrome)
    

    The same applies to Zipfile.

    The advantage, beyond brevity, is that regardless of what happens, the file will always be closed.

  • Use string formatting only when needed:

    # old
    'Download version ' + ver
    
    # new
    'Download version %s' % ver
    

    if you insist on using string formatting, use the newer form:

    'Download version {}'.format(ver)
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.