mysociety / yournextrepresentative

A website for crowd-sourcing structured election candidate data
https://candidates.democracyclub.org.uk/
GNU Affero General Public License v3.0
56 stars 21 forks source link

Fix migrations for Python 3 #858

Closed wfdd closed 8 years ago

wfdd commented 8 years ago

makemigrations fails in Python 3 if migrations contain both bytes and Unicode strings.

Closes #847.

mhl commented 8 years ago

I was testing this out, but I still get an error when trying to run makemigrations under Python 3. To try it out I was doing the following:

Indeed, I get this error:

Traceback (most recent call last):
  File "./manage.py", line 45, in <module>
    execute_from_command_line(sys.argv)
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/core/management/__init__.py", line 354, in execute_from_command_line
    utility.execute()
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/core/management/__init__.py", line 346, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/core/management/base.py", line 394, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/core/management/base.py", line 445, in execute
    output = self.handle(*args, **options)
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/core/management/commands/makemigrations.py", line 125, in handle
    migration_name=self.migration_name,
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/db/migrations/autodetector.py", line 43, in changes
    changes = self._detect_changes(convert_apps, graph)
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/db/migrations/autodetector.py", line 186, in _detect_changes
    self.generate_altered_fields()
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/db/migrations/autodetector.py", line 850, in generate_altered_fields
    if old_field_dec != new_field_dec:
  File "/home/mark/.virtualenvs/ynr3/lib/python3.4/site-packages/django/core/validators.py", line 55, in __eq__
    self.regex.pattern == other.regex.pattern and
AttributeError: 'bytes' object has no attribute 'pattern'

That's consistent with what I'd expect from this Django bug report and this StackOverflow question

However, then I cherry-picked 68c223a and re-ran ./manage.py makemigrations and got the same error again. I can't find any more byte strings in migrations from:

for d in `find . -type d -name migrations`; do egrep "\bb['\"]" "$d"/*.py; done

... either (in case some more had been introduced since your commit). Any ideas?

wfdd commented 8 years ago

@mhl: Have you updated the django-popolo migrations? See mysociety/django-popolo#5.

mhl commented 8 years ago

OK, great, it works fine with that change too. We should, of course, merge that pull request into the mySociety fork of django-popolo soon, but in the mean time I've pushed your branch to our fork, so if you could add something like the following to requirements.txt I'd be happy to merge this:

diff --git a/requirements.txt b/requirements.txt
index dcfe466..7737ded 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -21,7 +21,7 @@ django-model-utils==2.3.1
 django-nose==1.4.1
 django-notifications-hq==1.0.0
 django-pipeline==1.6.8
--e git+https://github.com/mysociety/django-popolo@update-with-better-migrations-redux#egg=mysociety-django-popolo
+-e git+https://github.com/mysociety/django-popolo@fix-migrations-for-python-3#egg=mysociety-django-popolo
 django-statici18n==1.1.5
 django-webtest==1.7.7
 djangorestframework==3.3.3

One other thing - it'd be good if you could add a bit more detail to the commit message, to help other people looking through the git history in future. e.g. as in your PR against django-popolo, you could include the links to the Django docs and some explanation - I'd also probably include the error message that you get when running makemigrations under Python 3.

wfdd commented 8 years ago

I'll do that. I'm assuming you made a copy of my PR branch at 'fix-migrations-for-python-3'?

wfdd commented 8 years ago

Okay, there was a merge conflict, so I've rebased, explained the original commit in some more detail and made the change to the requirements file.

mhl commented 8 years ago

Thanks, @wfdd :+1: