1
\$\begingroup\$

I want to write a script that imports data into a database (in django). The database has a Company class with a name (CharField) instance. Additionally, it has a Person class with name (CharField), email address (CharField) and a company which is a reference to the Company class (ForeginKey).

The database already has entries.

The script is importing the data from a csv file with the following format:

Company Name | Email Address | First Name | Last Name

While importing the data I want the script to go through the database and check if the same people only exist once (no duplicates) and also make sure that the email address is unique. So one email address can only belong to one person not multiple. Incorrections that are already in the database can be ignored.

def handle(self, *args, **options):
        with open('/path/to/myFile.csv') as f:  
            reader = csv.reader(f)
            next(f)
            for row in reader:
                company_in_list = row[0].strip()        ###reading the company name from the file
                visitor_company = ''
                company_in_db = Company.objects.filter(name=company_in_list)        ###searching for this company name in the db
                time.sleep(0.05)
                if len(company_in_db) == 1:                                     
                    visitor_company = company_in_db[0]
                    print "company already created", visitor_company
                elif len(company_in_db) == 0:                                       ###if the company doens't exist it will create one
                    print "created a new company"
                    visitor_company = Company.objects.create(name=company_in_list)
                else:
                    raise NameError("Multiple Companies with the same name, please fix the database")
                visitor_name = row[2].strip() + ' ' + row[3].strip()                ###reading the name of the person from the file
                email = row[1].strip()                                              ###reading the email address from the file
                person_in_db = Person.objects.filter(email=email)                   ###searching for this email address in the db   
                if len(person_in_db) == 0 :
                    Person.objects.create(company=visitor_company,name=visitor_name,email=email)    ###if email doesn't exist, create the person 
                    print "new Person created", visitor_name
                if len(person_in_db) > 0 :
                    for p in person_in_db:
                        if p.name == visitor_name and p.company == visitor_company:                 ###if the person does exist, if it's not the same one (different name or company) stop and raise an error
                            print "Person already created"
                            pass
                        else:
                            raise NameError('multiple people with the same email address, please fix this. Row:', row, p)
        print 'done commands'

My script opens the file, ignores the header row next(f), then for each row it searches the name of the company in the database. If it already exists, it will take that instance. If not, it will create that company. If there are multiple companies with the same name, the script throws an error.

Then it reads the email address and name from the file. It searches for the email address in the database. If it doesn't exist it creates an this person with the previously gathered infos (name and company). If it exists, it checks if the person in the database has the same name and company. If the info matches it will pass as the person is already in the database, if not it will raise an error saying there are multiple persons in the database.

The time.sleep() are in there so that the database won't get locked.

Is there any way to make this faster or write this better?

\$\endgroup\$
  • 1
    \$\begingroup\$ What is the purpose of the time.sleep(0.05)? \$\endgroup\$ – Austin Hastings Sep 22 '17 at 19:48
  • \$\begingroup\$ in the past I had problems with the database getting locked if the csv file had a lot of rows. So with the time.sleep(0.05) I avoided this. \$\endgroup\$ – Tom Sep 22 '17 at 19:55
  • 1
    \$\begingroup\$ Is that specific to the Company table, or just a general database-connection thing? \$\endgroup\$ – Austin Hastings Sep 22 '17 at 20:08
  • 1
    \$\begingroup\$ Also, what version of django? \$\endgroup\$ – Austin Hastings Sep 22 '17 at 20:12
  • \$\begingroup\$ I think it's a general database connection thing, true, I forgot to put that after the second filter, will change it thanks. I am using Django==1.10.3 \$\endgroup\$ – Tom Sep 22 '17 at 21:07
1
\$\begingroup\$

You don't specify how many new records you're adding. But you did mention in comments some trouble with the "database getting locked", which I'll interpret as "database bogging down because you were hammering it with this script", and so I'll assume you're inserting a lot of records.

If you're doing that, and you're not stuck on an old version of Django, there are some bulk data commands to help you. More on that later.

First, though, let's talk about your constraints. You are applying a very strict uniqueness constraint to company names: a company name is assumed to be totally distinct.

I don't know your context, but I'm from the United States. In the US, corporations are managed at the state level: you'll hear things like "ExampleCorp, a Delaware corporation, ..."

Also, businesses other than corporations are managed at the county or city level (depending on the state, and the size of the city). So I can go down to my "county seat" (the town where the business of the county is conducted, usually containing the jail and the county courthouse) and register a "DBA form" (DBA means "doing business as") that creates a "name" for my business.

So you might go to your county seat in county 1, and register "TomCo". And someone else named Tom might go to the county seat in county 2 and register "TomCo". And they would be totally legit.

So, again, I don't know your use case. But you might need to re-think the whole "company names are distinct" thing. Perhaps you could attach an email domain, which is guaranteed to be globally unique, to the company record?

Regardless, there's also the chance that your batch of users might include the same company multiple times. For example, if your Django app is some sort of SaaS and you are bulk-loading a new customer, maybe all the new people are from the same company. So I think it behooves you to cache that information.

Next, there's the chance that your users are in the input file more than once. So you might as well filter those out.

Next, there's the concept that your users must have a "first name" and a "last name" and that's it. Seriously? That's very Anglo-centric, but not very realistic. What about people from Brazil, who generally go by a single name, like "Ronaldinho"? (Although they come from the Portuguese tradition, so they might have 4 names if they get formal: "Ronaldo de Assis Moreira"?) What about my all-time favorite bandit, "Tuco Benedicto Pacifico Juan Maria Ramirez"? At least your database uses a single name field, so that's good.

And finally, you should process the new user creations in bulk if possible, to minimize the number of database calls and the number of transactions.

Let's take a look:

def handle(self, *args, **options):
    with open('/path/to/myFile.csv') as f:  
        reader = csv.reader(f)
        next(f)

The only problem with the first few lines is the hard-coding of the csv file. Is there no way to get that into a command line argument, or configuration setting?

        for row in reader:
            company_in_list = row[0].strip()        ###reading the company name from the file
            visitor_company = ''
            company_in_db = Company.objects.filter(name=company_in_list)        ###searching for this company name in the db
            time.sleep(0.05)

This section I object to because you're doing things in a scattershot order. There are several places where you pull out fields from the csv row, and you always use the all the fields, except when you raise an exception and abort. So let's put all the fields together:

for row in reader:
    company = row[0].strip()
    email   = row[1].strip()
    first   = row[2].strip()
    last    = row[3].strip()

But we can do better! That's just looping over a bunch of things, performing the exact same computation for each: we could use a list comprehension or a generator expression, like so:

for row in reader:
    (co_name, email, first, last) = (field.strip() for field in row)

Your CSV file may actually have more columns than 4, so you might have to use row[:4] instead.

First, let's deal with the name. And let's support mononymy! I'm doing this because an invalid name seems like a good reason to skip the record, which we should do as early as possible:

    name = last if not first
        else first if not last
        else first + ' ' + last

    if not name:
        print("Skipping bad names first={!r}, last={!r}".format(first, last))

Next, you check if the company is already in the database. If it's in the database, great. If it's not in the database, you create it. If it's in the database more than once, you raise an exception.

I don't think that last case is really the job of this script. That condition - that the company name should be unique in the table - is properly the job of a database constraint rather than a "create users" script. (It's not even a "create companies" script, which would at least be on-topic.)

So I'm going to suggest you ignore that case. Let's just have the script either create the company if it doesn't exist, or use the existing company if there is already a record.

One thing you don't do, is check if the company name is empty. I don't know if this is a valid case or not, so I'm going to leave that alone (after bringing it to your attention).

Our first step will be to separate out the company names (because there could be M:N of them versus the people), and the email addresses (because that is your key, apparently).

Let's put the company names into a set, in order to make them unique. And we'll put the entire record into a dictionary keyed by the email addresses:

companies = set()
visitors = {} # email -> tuple

for row in ...

    # ...

    companies.add(co_name)

    v = (email, name, co_name)

    if email in visitors:
        print('''Duplicate emails found:
{email} -> {old}
and 
{email} -> {new}'''
            .format(email=email, old=visitors[email], new=v))

     visitors[email] = v

At this point, after looping through all the records, you will have a set of company names and a dictionary of visitors, with all the duplicate company names merged in the set, and all the duplicate emails merged in the dictionary.

Now you can process your companies as a single query:

existing_companies = Company.objects.filter(name__in=companies)

That should be a single database call.

You'll want to compute the set of companies that are not in the existing_companies result set, and then bulk_create them:

new_companies = ...

Company.objects.bulk_create([Company(name=cn) for cn in new_companies])

That should be a single database call.

Finally, you have to deal with the users. You existing code doesn't seem to allow for changing jobs: if you have Fred.Flintstone@yahoo.com working at Slate Rock and Gravel, there's no way for him to change jobs to Spacely Sprockets. Trying to change the company associated with a visitor's email will result in an error. I don't know if that's because of what you're doing, or if it's an oversight. You'll have to make the call.

At any rate, you can do a similar thing with querying for email addresses already in the Person table, computing the set of emails not in the table, and doing a bulk_create on those records. (You'll want to create a dictionary to map company names to records, first, using the existing_companies results, plus maybe another query after the bulk create, or a rerun of the whole companies query.)

Again, that should be a single database call. But this depends on your underlying database. The Django docs warn that there are limits on SQLite, so you may have to do your bulk creates in batches.

Summary

I believe you can improve your database availability, and the script performance, by doing the following:

  1. Read all the CSV records into memory (companies, visitors). Do some basic consistency checking during this part.

  2. Make a single bulk query for existing companies.

  3. Perform a single bulk create for new companies.

  4. Refresh the companies query.

  5. Perform a bulk query for existing visitors.

  6. Perform a bulk create for new visitors.

That's potentially only 5 database calls.

\$\endgroup\$
  • \$\begingroup\$ Thanks you so much for the extensive answer, I worked most of the stuff in, but now I am stuck on creating the bulk create for the new visitors, in particular, I don't know how to get the companies for each person. \$\endgroup\$ – Tom Sep 26 '17 at 16:55
  • 1
    \$\begingroup\$ Refresh the query in step (4) of my Summary; that should give you a result set filled with company objects. Each new visitor has the company name in the incoming csv, which was how you made the list of companies. So find the company in the result set which has the matching name (build a map and process the result set one time), then do your new-user create with the company-object in the right field. \$\endgroup\$ – Austin Hastings Sep 26 '17 at 18:51

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.