Class for multithreaded insert into database Here I have class that supposed to take dictionaries of product objects and their articles (kind of unique identifier) from queue and insert or update them in database table.

Received dictionary looks like this:

products_and_articles= {'products':[],'articles':[]}


Please suggest any improvements I can make to this class.

class TableUpdater(threading.Thread):

def __init__(self, import_queue, model_for_update):

    self.import_queue = import_queue
    self.model_for_update = model_for_update
    threading.Thread.__init__(self)

def run(self):
    while True:
        try:
            products_and_articles = self.import_queue.get(block=True, timeout=20)

            products_for_update = self.model_for_update.objects.filter(article__in=products_and_articles['articles'])
            articles_for_update = [product.article for product in products_for_update]

            articles_for_create = list(set(products_and_articles['articles']) - set(articles_for_update))
            absent_products = [p for p in products_and_articles['products'] if p.article in articles_for_create]
            products_for_create = []

            for new_product in products_and_articles['products']:
                for old_product in products_for_update:
                    if new_product.article == old_product.article:
                        for old_field in old_product._meta.get_all_field_names():
                            for new_field in new_product._meta.get_all_field_names():
                                if old_field == new_field and old_field != 'id' and old_field != 'slug':
                                    setattr(old_product, old_field, getattr(new_product, old_field))

            bulk_update(products_for_update)

            if len(products_for_update) != len(products_and_articles['products']):
                for new_product in absent_products:
                    old_product = self.model_for_update()
                    for old_field in old_product._meta.get_all_field_names():
                        for new_field in new_product._meta.get_all_field_names():
                            if old_field == new_field and old_field != 'id':
                                setattr(old_product, old_field, getattr(new_product, old_field))
                    products_for_create.append(old_product)

                product_bulk_create(products_for_create, 0, articles_for_create)

        except Queue.Empty:
            print 'Queue is empty'
            break
As mentioned by jcfollower, you have two sections that are deeply indented. These should be extracted into sub-methods. It will make the code easier to follow and not cause the text to bunch up at the right side of the screen.

More importantly, it is currently hard to tell what is actually happening with all the for loops and if clauses. Well named methods will provide context to the reader. When I'm looking at run(), I'm not concerned with how each filed is updated. I just want to know the main tasks that are being done. Then, once I have a high level understanding, I can dive done into the implementation details to check if the updates are done correctly and efficiently1.



Specify the queue read timeout as a constant instead of a magic number. If you find the the value needs to be tuned, it will be easier to find.



self.model_for_update.objects.filter(article__in=products_and_articles['articles'])


Is the double underscore in the argument name intentional? Python has a convention for when double underscores are used and separating words in variable names is very different from the convention.



Does product_bulk_create() put some restriction on articles_for_create such that it must be a list? Unless that is the case, using the set returned by the set subtraction should be sufficient.



products_and_articles is a odd name for a variable. You never have an operation that directly uses the whole dictionary. You are just passing references of the respective lists. Instead, you can replace the dictionary with a tuple and unpack the values directly into variables.

products, articles = self.import_queue.get(block=True, timeout=20)




1 For loops that are nested 3 and 4 deep are flags that there might be an inefficient algorithm.