Download URL from clipboard This is a Python script I made to download a URL from the clipboard.

When a new URL is copied to a clipboard it adds the URL to a download queue, then on a separate thread it downloads the URL to a file. 

I am new to multithreading.  Is my script properly using multithreading? It needs to download the data while still listening for changes in the content of the clipboard. 

import time
import os
import urllib
import subprocess
import threading

download_number = 0
count=0
save_file = "links_list.txt"
old_url = ""
download_queue = []
download_delay = 5

data_file = open(save_file, "r")
past_links = data_file.read()
data_file.close()

def get_image():
    print download_queue
    url = download_queue.pop(0)

    data = urllib.urlopen(url).read()
    name = url.split("/")[-1:][0]
    extension = name.split(".")[-1:][0]
    basename = name.split(".")[:-1][0]

    # make a new file name
    if os.path.exists(name):
        new_name = basename+"-0."+extension
        try_count = 0
        while os.path.exists(new_name):
            try_count += 1
            new_name = basename+"-"+str(try_count)+"."+extension
        name = new_name

    data_file = open(name, "w")
    data_file.write(data)
    data_file.close()


    # goto the next link
    if len(download_queue) != 0:
        time.sleep(download_delay)
        get_image()


while(1):
    time.sleep(0.25)
    clipboard = subprocess.check_output('pbpaste', shell=True)
    # check if the clipbard contains a new url
    if clipboard[:4] == "http" and clipboard != old_url and clipboard not in past_links:
        past_links += clipboard
        old_url = clipboard

        # add url to queue
        download_queue.append(clipboard)
        print download_number,

        # start the download fuction
        t = threading.Thread(target=get_image)
        t.start()

        # links that have been downloaded
        data_file = open(save_file, "a")
        data_file.write(clipboard+"\n")
        data_file.close()

        download_number += 1
You maintain old_url as well as past_links. You probably don't need old_url at all because everything you process will be added to past_links.

You are reading past_links as one giant string with embedded newlines. That won't be very efficient when doing clipboard not in past_links because it will have to search through the whole string.

When you run past_links += clipboard, you are sticking the data on to the end of past_links with no newline at the end, so new links added there will be jammed together.

You should probably set up past_links as a set, and use set operations. You can still read and write it as a plain text file.

You create a new thread for each download request, which will download multiple files in parallel. It's not clear that is what you really want to do.

Your use of a recursive call to get_image() is a very strange way to implement a loop.

You are reading the entire download into memory in one giant string, and then writing it out to disk. If you were to download a very large file, your script would require a corresponding amount of memory during downloading. Use urllib.urlretrieve to read directly into a file.

Use clipboard.startswith(&quot;http&quot;) instead of clipboard[:4] == &quot;http&quot;

Finally, use the standard Python Queue module instead of implementing your own:

The Queue module implements multi-producer, multi-consumer queues. It is especially useful in threaded programming when information must be exchanged safely between multiple threads.