Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Senthil Kumaran S


On Wednesday 31 May 2017 03:00 AM, Brian May wrote:
> Raphael Hertzog  writes:
> 
>> Is that actually true? linaro_django_xmlrpc seems to be listed in
>> INSTALLED_APPS, no?
>>
>> Do you have any idea why it would give this error?
> 
> I note that linaro_django_xmlrpc is towards the end of
> INSTALLED_APPS. Maybe it did not get loaded?

Just to ensure, I changed the order as follows and tried, which didn't
help.


INSTALLED_APPS = [
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.sites',
'django.contrib.humanize',
'django.contrib.staticfiles',
'django.contrib.admin',
'linaro_django_xmlrpc',
# Uncomment the next line to enable admin documentation:

# 'django.contrib.admindocs',

# Add LAVA applications

'dashboard_app',
'lava_results_app',
'lava_scheduler_daemon',
'lava_scheduler_app',
# Needed applications

'django_tables2',
'google_analytics',
]


Traceback available here - http://paste.debian.net/954509/

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/
http://www.sasenthilkumaran.com/



signature.asc
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Brian May
Raphael Hertzog  writes:

> Is that actually true? linaro_django_xmlrpc seems to be listed in
> INSTALLED_APPS, no?
>
> Do you have any idea why it would give this error?

I note that linaro_django_xmlrpc is towards the end of
INSTALLED_APPS. Maybe it did not get loaded?

Maybe it stopped loading apps the moment it detects a bad dependancy?

After looking at the code, this doesn't exactly make sense, but maybe
something here might help regardless.
-- 
Brian May 



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Senthil Kumaran S


On Tuesday 30 May 2017 08:39 PM, Raphael Hertzog wrote:
> On Tue, 30 May 2017, Senthil Kumaran S wrote:
>> I tested the new version ie., test2 and got a traceback as shown here -
>>   File "/usr/lib/python2.7/dist-packages/django/db/migrations/state.py",
>> line 249, in __init__
>> raise ValueError("\n".join(error.msg for error in errors))
>> ValueError: The field lava_scheduler_app.TestJob.submit_token was
>> declared with a lazy reference to 'linaro_django_xmlrpc.authtoken', but
>> app 'linaro_django_xmlrpc' isn't installed.
> 
> Is that actually true? linaro_django_xmlrpc seems to be listed in
> INSTALLED_APPS, no?

Yes it is listed in INSTALLED_APPS. From the code:


INSTALLED_APPS = [
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.sites',
'django.contrib.humanize',
'django.contrib.staticfiles',
'django.contrib.admin',
# Uncomment the next line to enable admin documentation:

# 'django.contrib.admindocs',

# Add LAVA applications

'lava_server',
'dashboard_app',
'lava_results_app',
'lava_scheduler_daemon',
'lava_scheduler_app',
# Needed applications

'django_tables2',
'linaro_django_xmlrpc',
'google_analytics',
]


> Do you have any idea why it would give this error?

Since I am not very proficient with django core / migration code, I do
not know the exact reason. But following is what I understand.

linaro_django_xmlrpc is listed as a dependency in lava_scheduler_app's
initial migration. But it is not honored by the dependency graph that is
generated, where lava_scheduler_app will already be marked as migrated.
linaro_django_xmlrpc should be faked during this migration which we are
unable to do due to the check introduced in 1.10.

To give more background. The migration for linaro_django_xmlrpc was
introduced in November 2015 -
https://git.linaro.org/lava/lava-server.git/commit/?id=02a7c3508c01b89c7efb5d59e79b6880a229ff14

linaro_django_xmlrpc will already be available in the database, but it
was not recorded in the migrations table which should be faked now,
since this migration was introduced after creating the necessary tables
in the database.

> Your patch is a work-around and it works without my patch, so it's
> expected that it would also work with mine... but I would like a Django
> fix that would not require any further change in lava.

I agree.

Thank You.
-- 
Senthil Kumaran
http://www.stylesen.org/
http://www.sasenthilkumaran.com/



signature.asc
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Raphael Hertzog
Hi,

On Tue, 30 May 2017, Senthil Kumaran S wrote:
> I tested the new version ie., test2 and got a traceback as shown here -
>   File "/usr/lib/python2.7/dist-packages/django/db/migrations/state.py",
> line 249, in __init__
> raise ValueError("\n".join(error.msg for error in errors))
> ValueError: The field lava_scheduler_app.TestJob.submit_token was
> declared with a lazy reference to 'linaro_django_xmlrpc.authtoken', but
> app 'linaro_django_xmlrpc' isn't installed.

Is that actually true? linaro_django_xmlrpc seems to be listed in
INSTALLED_APPS, no?

Do you have any idea why it would give this error?

> When combined with the attached patch for lava-server the migration
> works fine as seen here - http://paste.debian.net/952953/

Your patch is a work-around and it works without my patch, so it's
expected that it would also work with mine... but I would like a Django
fix that would not require any further change in lava.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Senthil Kumaran S


On Tuesday 30 May 2017 06:09 PM, Senthil Kumaran S wrote:
> I tested the new version ie., test2 and got a traceback as shown here -
> http://paste.debian.net/952939/

When combined with the attached patch for lava-server the migration
works fine as seen here - http://paste.debian.net/952953/

Thank You.
-- 
Senthil Kumaran
http://www.stylesen.org/
http://www.sasenthilkumaran.com/
From dda5f10642678d769ca300325feae771b30baceb Mon Sep 17 00:00:00 2001
From: Senthil Kumaran S 
Date: Mon, 29 May 2017 17:39:39 +0530
Subject: [PATCH] Fix migrations from Debian Jessie to Stretch and ensure smooth upgrade.

Django 1.10.1 introduces consistency checks which introduces dependency
problem with the order in which lava_scheduler_app and linaro_django_xmlrpc
initial migrations gets applied.

Related bugs:
* Debian:
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863267
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=847277

* Django:
  https://code.djangoproject.com/ticket/28250

Change-Id: I4ea7632d9e98d6fd5e01aca65c37410ce7dd628a
---

diff --git a/lava_scheduler_app/migrations/0001_initial.py b/lava_scheduler_app/migrations/0001_initial.py
index e321682..eda8572 100644
--- a/lava_scheduler_app/migrations/0001_initial.py
+++ b/lava_scheduler_app/migrations/0001_initial.py
@@ -1,12 +1,26 @@
 # -*- coding: utf-8 -*-
 from __future__ import unicode_literals
 
-from django.db import models, migrations
+from django.db import models, migrations, DEFAULT_DB_ALIAS, connections
+from django.db.migrations.recorder import MigrationRecorder
 import django.db.models.deletion
 from django.conf import settings
 import lava_scheduler_app.models
 
 
+connection = connections[DEFAULT_DB_ALIAS]
+recorder = MigrationRecorder(connection)
+linaro_django_xmlrpc_applied = False
+lava_scheduler_app_applied = False
+for app, name in recorder.applied_migrations():
+if app == 'linaro_django_xmlrpc' and name == '0001_initial':
+linaro_django_xmlrpc_applied = True
+if app == 'lava_scheduler_app' and name == '0001_initial':
+lava_scheduler_app_applied = True
+if not linaro_django_xmlrpc_applied and lava_scheduler_app_applied:
+recorder.record_applied('linaro_django_xmlrpc', '0001_initial')
+
+
 class Migration(migrations.Migration):
 
 dependencies = [


signature.asc
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Senthil Kumaran S


On Tuesday 30 May 2017 05:15 PM, Raphael Hertzog wrote:
> Thanks, can you try again with another test version ?
> $ dget 
> https://people.debian.org/~hertzog/packages/python-django_1.10.7-2~test2_amd64.changes
> (test2 now, no longer test1)

I tested the new version ie., test2 and got a traceback as shown here -
http://paste.debian.net/952939/

>> AttributeError: 'Node' object has no attribute 'initial'
> 
> Upstream helped me to hopefully fix this error.

Yes it gets past the above.

Traceback observed:


Traceback (most recent call last):
  File "/usr/bin/lava-server", line 78, in 
main()
  File "/usr/bin/lava-server", line 74, in main
execute_from_command_line(django_options)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/__init__.py",
line 367, in execute_from_command_line
utility.execute()
  File
"/usr/lib/python2.7/dist-packages/django/core/management/__init__.py",
line 359, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/base.py", line
294, in run_from_argv
self.execute(*args, **cmd_options)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/base.py", line
345, in execute
output = self.handle(*args, **options)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/commands/migrate.py",
line 165, in handle
pre_migrate_apps = pre_migrate_state.apps
  File "/usr/lib/python2.7/dist-packages/django/utils/functional.py",
line 35, in __get__
res = instance.__dict__[self.name] = self.func(instance)
  File "/usr/lib/python2.7/dist-packages/django/db/migrations/state.py",
line 176, in apps
return StateApps(self.real_apps, self.models)
  File "/usr/lib/python2.7/dist-packages/django/db/migrations/state.py",
line 249, in __init__
raise ValueError("\n".join(error.msg for error in errors))
ValueError: The field lava_scheduler_app.TestJob.submit_token was
declared with a lazy reference to 'linaro_django_xmlrpc.authtoken', but
app 'linaro_django_xmlrpc' isn't installed.


Thank You.
-- 
Senthil Kumaran
http://www.stylesen.org/
http://www.sasenthilkumaran.com/



signature.asc
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Raphael Hertzog
On Tue, 30 May 2017, Senthil Kumaran S wrote:
> I tested the patch with lava-server, which ended up with a traceback as
> seen here - http://paste.debian.net/952276/

Thanks, can you try again with another test version ?
$ dget 
https://people.debian.org/~hertzog/packages/python-django_1.10.7-2~test2_amd64.changes
(test2 now, no longer test1)

> AttributeError: 'Node' object has no attribute 'initial'

Upstream helped me to hopefully fix this error.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Senthil Kumaran S
On Monday 29 May 2017 08:54 PM, Raphael Hertzog wrote:
> On Mon, 29 May 2017, Raphael Hertzog wrote:
>> Updated patches attached, I missed to update some tests to account
>> for the move of the detect_soft_applied() method.
> 
> Third set of patches, this time the package builds fine at least.
> Which means you can just test this package and let me know if it fixes
> your issue:
> $ dget 
> https://people.debian.org/~hertzog/packages/python-django_1.10.7-2~test1_amd64.changes

I tested the patch with lava-server, which ended up with a traceback as
seen here - http://paste.debian.net/952276/

The traceback message is as follows:


Traceback (most recent call last):
  File "/usr/bin/lava-server", line 78, in 
main()
  File "/usr/bin/lava-server", line 74, in main
execute_from_command_line(django_options)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/__init__.py",
line 367, in execute_from_command_line
utility.execute()
  File
"/usr/lib/python2.7/dist-packages/django/core/management/__init__.py",
line 359, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/base.py", line
294, in run_from_argv
self.execute(*args, **cmd_options)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/base.py", line
345, in execute
output = self.handle(*args, **options)
  File
"/usr/lib/python2.7/dist-packages/django/core/management/commands/migrate.py",
line 87, in handle
connection, fake_initial=options['fake_initial'])
  File
"/usr/lib/python2.7/dist-packages/django/db/migrations/loader.py", line
291, in check_consistent_history
if fake_initial and self.detect_soft_applied(None, parent):
  File
"/usr/lib/python2.7/dist-packages/django/db/migrations/loader.py", line
343, in detect_soft_applied
if migration.initial is None:
AttributeError: 'Node' object has no attribute 'initial'


Thank You.
-- 
Senthil Kumaran
http://www.stylesen.org/
http://www.sasenthilkumaran.com/



signature.asc
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-30 Thread Senthil Kumaran S


On Monday 29 May 2017 08:54 PM, Raphael Hertzog wrote:
> On Mon, 29 May 2017, Raphael Hertzog wrote:
>> Updated patches attached, I missed to update some tests to account
>> for the move of the detect_soft_applied() method.
> 
> Third set of patches, this time the package builds fine at least.
> Which means you can just test this package and let me know if it fixes
> your issue:
> $ dget 
> https://people.debian.org/~hertzog/packages/python-django_1.10.7-2~test1_amd64.changes

I will test this with lava-server and let you know the results.

Thank You.
-- 
Senthil Kumaran
http://www.stylesen.org/
http://www.sasenthilkumaran.com/



signature.asc
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-29 Thread Raphael Hertzog
On Mon, 29 May 2017, Raphael Hertzog wrote:
> Updated patches attached, I missed to update some tests to account
> for the move of the detect_soft_applied() method.

Third set of patches, this time the package builds fine at least.
Which means you can just test this package and let me know if it fixes
your issue:
$ dget 
https://people.debian.org/~hertzog/packages/python-django_1.10.7-2~test1_amd64.changes

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/
>From c6d66195d7f816aeb47a77570bdd3836a99d4183 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= 
Date: Mon, 29 May 2017 15:44:39 +0200
Subject: [PATCH 1/2] Move detect_soft_applied() from
 django.db.migrations.executor to .loader
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We want to be able to use that method in
loader.check_consistent_history() to accept an history where the initial
migration is going to be fake-applied. Since the executor has the
knowledge of the loader (but not the opposite), it makes sens to move
the code around.

Signed-off-by: Raphaël Hertzog 
---
 django/db/migrations/executor.py  | 83 +--
 django/db/migrations/loader.py| 81 ++
 tests/migrations/test_executor.py | 12 +++---
 3 files changed, 88 insertions(+), 88 deletions(-)

diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py
index 1a0b6f6322..2ac787b0b2 100644
--- a/django/db/migrations/executor.py
+++ b/django/db/migrations/executor.py
@@ -1,8 +1,5 @@
 from __future__ import unicode_literals
 
-from django.apps.registry import apps as global_apps
-from django.db import migrations, router
-
 from .exceptions import InvalidMigrationPlan
 from .loader import MigrationLoader
 from .recorder import MigrationRecorder
@@ -235,7 +232,7 @@ class MigrationExecutor(object):
 if not fake:
 if fake_initial:
 # Test to see if this is an already-applied initial migration
-applied, state = self.detect_soft_applied(state, migration)
+applied, state = self.loader.detect_soft_applied(state, migration)
 if applied:
 fake = True
 if not fake:
@@ -290,81 +287,3 @@ class MigrationExecutor(object):
 if all_applied and key not in applied:
 self.recorder.record_applied(*key)
 
-def detect_soft_applied(self, project_state, migration):
-"""
-Tests whether a migration has been implicitly applied - that the
-tables or columns it would create exist. This is intended only for use
-on initial migrations (as it only looks for CreateModel and AddField).
-"""
-def should_skip_detecting_model(migration, model):
-"""
-No need to detect tables for proxy models, unmanaged models, or
-models that can't be migrated on the current database.
-"""
-return (
-model._meta.proxy or not model._meta.managed or not
-router.allow_migrate(
-self.connection.alias, migration.app_label,
-model_name=model._meta.model_name,
-)
-)
-
-if migration.initial is None:
-# Bail if the migration isn't the first one in its app
-if any(app == migration.app_label for app, name in migration.dependencies):
-return False, project_state
-elif migration.initial is False:
-# Bail if it's NOT an initial migration
-return False, project_state
-
-if project_state is None:
-after_state = self.loader.project_state((migration.app_label, migration.name), at_end=True)
-else:
-after_state = migration.mutate_state(project_state)
-apps = after_state.apps
-found_create_model_migration = False
-found_add_field_migration = False
-existing_table_names = self.connection.introspection.table_names(self.connection.cursor())
-# Make sure all create model and add field operations are done
-for operation in migration.operations:
-if isinstance(operation, migrations.CreateModel):
-model = apps.get_model(migration.app_label, operation.name)
-if model._meta.swapped:
-# We have to fetch the model to test with from the
-# main app cache, as it's not a direct dependency.
-model = global_apps.get_model(model._meta.swapped)
-if should_skip_detecting_model(migration, model):
-continue
-if model._meta.db_table not in existing_table_names:
-return False, project_state

Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-29 Thread Raphael Hertzog
On Mon, 29 May 2017, Raphael Hertzog wrote:
> Option 4. Fix Django 1.10 with the attached patches.

Updated patches attached, I missed to update some tests to account
for the move of the detect_soft_applied() method.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/
>From 180e96bfac0647c2b10b11123c7f6147a2518373 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= 
Date: Mon, 29 May 2017 15:44:39 +0200
Subject: [PATCH 1/2] Move detect_soft_applied() from
 django.db.migrations.executor to .loader
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We want to be able to use that method in
loader.check_consistent_history() to accept an history where the initial
migration is going to be fake-applied. Since the executor has the
knowledge of the loader (but not the opposite), it makes sens to move
the code around.

Signed-off-by: Raphaël Hertzog 
---
 django/db/migrations/executor.py  | 81 +--
 django/db/migrations/loader.py| 80 ++
 tests/migrations/test_executor.py | 12 +++---
 3 files changed, 87 insertions(+), 86 deletions(-)

diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py
index 1a0b6f6322..ed5b64db60 100644
--- a/django/db/migrations/executor.py
+++ b/django/db/migrations/executor.py
@@ -1,7 +1,6 @@
 from __future__ import unicode_literals
 
 from django.apps.registry import apps as global_apps
-from django.db import migrations, router
 
 from .exceptions import InvalidMigrationPlan
 from .loader import MigrationLoader
@@ -235,7 +234,7 @@ class MigrationExecutor(object):
 if not fake:
 if fake_initial:
 # Test to see if this is an already-applied initial migration
-applied, state = self.detect_soft_applied(state, migration)
+applied, state = self.loader.detect_soft_applied(state, migration)
 if applied:
 fake = True
 if not fake:
@@ -290,81 +289,3 @@ class MigrationExecutor(object):
 if all_applied and key not in applied:
 self.recorder.record_applied(*key)
 
-def detect_soft_applied(self, project_state, migration):
-"""
-Tests whether a migration has been implicitly applied - that the
-tables or columns it would create exist. This is intended only for use
-on initial migrations (as it only looks for CreateModel and AddField).
-"""
-def should_skip_detecting_model(migration, model):
-"""
-No need to detect tables for proxy models, unmanaged models, or
-models that can't be migrated on the current database.
-"""
-return (
-model._meta.proxy or not model._meta.managed or not
-router.allow_migrate(
-self.connection.alias, migration.app_label,
-model_name=model._meta.model_name,
-)
-)
-
-if migration.initial is None:
-# Bail if the migration isn't the first one in its app
-if any(app == migration.app_label for app, name in migration.dependencies):
-return False, project_state
-elif migration.initial is False:
-# Bail if it's NOT an initial migration
-return False, project_state
-
-if project_state is None:
-after_state = self.loader.project_state((migration.app_label, migration.name), at_end=True)
-else:
-after_state = migration.mutate_state(project_state)
-apps = after_state.apps
-found_create_model_migration = False
-found_add_field_migration = False
-existing_table_names = self.connection.introspection.table_names(self.connection.cursor())
-# Make sure all create model and add field operations are done
-for operation in migration.operations:
-if isinstance(operation, migrations.CreateModel):
-model = apps.get_model(migration.app_label, operation.name)
-if model._meta.swapped:
-# We have to fetch the model to test with from the
-# main app cache, as it's not a direct dependency.
-model = global_apps.get_model(model._meta.swapped)
-if should_skip_detecting_model(migration, model):
-continue
-if model._meta.db_table not in existing_table_names:
-return False, project_state
-found_create_model_migration = True
-elif isinstance(operation, migrations.AddField):
-model = apps.get_model(migration.app_label, operation.model_name)
-if 

Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-29 Thread Raphael Hertzog
On Mon, 29 May 2017, Brian May wrote:
> Otherwise, I think we have three options. I recommend reading the Django
> ticket in full before deciding. 
[…]
> 1. Apply work around from
> https://code.djangoproject.com/ticket/28250#comment:1 by manually
[…]
> 2. Remove migration from postinst, and give instructions for manually
> updating the database. Modify
[…]
> 3. Drop lava-server from testing. 
[…]

Option 4. Fix Django 1.10 with the attached patches.

I don't have time right now to test them, but I would love if someone else
could try them... the idea is to not barf on the inconsistent history if
we detect that the missing migration can be fake-applied.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/
>From ee93aeecc298f801b85cd49366e5a431d1867f0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= 
Date: Mon, 29 May 2017 15:44:39 +0200
Subject: [PATCH 1/2] Move detect_soft_applied() from
 django.db.migrations.executor to .loader
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We want to be able to use that method in
loader.check_consistent_history() to accept an history where the initial
migration is going to be fake-applied. Since the executor has the
knowledge of the loader (but not the opposite), it makes sens to move
the code around.

Signed-off-by: Raphaël Hertzog 
---
 django/db/migrations/executor.py | 81 +---
 django/db/migrations/loader.py   | 80 +++
 2 files changed, 81 insertions(+), 80 deletions(-)

diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py
index 1a0b6f6322..ed5b64db60 100644
--- a/django/db/migrations/executor.py
+++ b/django/db/migrations/executor.py
@@ -1,7 +1,6 @@
 from __future__ import unicode_literals
 
 from django.apps.registry import apps as global_apps
-from django.db import migrations, router
 
 from .exceptions import InvalidMigrationPlan
 from .loader import MigrationLoader
@@ -235,7 +234,7 @@ class MigrationExecutor(object):
 if not fake:
 if fake_initial:
 # Test to see if this is an already-applied initial migration
-applied, state = self.detect_soft_applied(state, migration)
+applied, state = self.loader.detect_soft_applied(state, migration)
 if applied:
 fake = True
 if not fake:
@@ -290,81 +289,3 @@ class MigrationExecutor(object):
 if all_applied and key not in applied:
 self.recorder.record_applied(*key)
 
-def detect_soft_applied(self, project_state, migration):
-"""
-Tests whether a migration has been implicitly applied - that the
-tables or columns it would create exist. This is intended only for use
-on initial migrations (as it only looks for CreateModel and AddField).
-"""
-def should_skip_detecting_model(migration, model):
-"""
-No need to detect tables for proxy models, unmanaged models, or
-models that can't be migrated on the current database.
-"""
-return (
-model._meta.proxy or not model._meta.managed or not
-router.allow_migrate(
-self.connection.alias, migration.app_label,
-model_name=model._meta.model_name,
-)
-)
-
-if migration.initial is None:
-# Bail if the migration isn't the first one in its app
-if any(app == migration.app_label for app, name in migration.dependencies):
-return False, project_state
-elif migration.initial is False:
-# Bail if it's NOT an initial migration
-return False, project_state
-
-if project_state is None:
-after_state = self.loader.project_state((migration.app_label, migration.name), at_end=True)
-else:
-after_state = migration.mutate_state(project_state)
-apps = after_state.apps
-found_create_model_migration = False
-found_add_field_migration = False
-existing_table_names = self.connection.introspection.table_names(self.connection.cursor())
-# Make sure all create model and add field operations are done
-for operation in migration.operations:
-if isinstance(operation, migrations.CreateModel):
-model = apps.get_model(migration.app_label, operation.name)
-if model._meta.swapped:
-# We have to fetch the model to test with from the
-# main app cache, as it's not a direct dependency.
-model = global_apps.get_model(model._meta.swapped)
-if 

Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-28 Thread Brian May
On 2017-05-26 19:05, Neil Williams wrote:

> No. This is django making the wrong decision about problems it has
> previously supported when trying to include bug fixes for other
> problems. It is a regression in django 1.10

Feel free to argue this point in
https://code.djangoproject.com/ticket/28250 

Otherwise, I think we have three options. I recommend reading the Django
ticket in full before deciding. 

1. Apply work around from
https://code.djangoproject.com/ticket/28250#comment:1 by manually
recording that the database transaction has been applied. This could
potentially be automated as part of the postinst. 

2. Remove migration from postinst, and give instructions for manually
updating the database. Modify
/usr/share/doc/python-django/examples/migrate-south (maybe create new
file in Django package) so instead of installing Django 1.6.x in a
virtualenv and running migrations, it installs Django 1.8.x into
virtualenv and runs migrations. 

https://anonscm.debian.org/cgit/python-modules/packages/python-django.git/tree/debian/contrib/migrate-south


3. Drop lava-server from testing. 

Also it might be worth updating the migrate script in Django - as per
option 2 - even if you decide 1 is the best option. Would help in case
any similar problems are identified - maybe with packages not in Debian
- after the release.

Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-28 Thread Brian May
control: forwarded -1 https://code.djangoproject.com/ticket/28250#ticket

Brian May  writes:

> B. Create a Django bug report pointing to our test case. They may or may
> not accept it as a bug in Django, however it would be good to get their
> feedback.

Done: https://code.djangoproject.com/ticket/28250

Hopefully I have described the problem clearly and accurately.
-- 
Brian May 



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Brian May
Some misconceptions resolved:

* This bug does not cause any data to be deleted.
* This bug does not cause any data to be currupted.
* This bug does not cause any data to be lost.
* This bug only affects one application. Out of many others that use Django.

The damage this bug does cause:

* If you try to upgrade the package in question, the migrations will run
  in the postinst script. These migrations will fail - before even
  touching the database. This is a pre-migration sanity check that
  fails. This in turn means the postinst script will fail and the
  package is marked as bad.

  All the data is still there, and it hasn't been touched. It may not be
  accessible to the application however, as it will depend on the
  migrations being run. The application may crash as the database schema
  is different from what it expects.

I do not believe this should be an RC bug against Django.
-- 
Brian May 



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Brian May
Neil Williams  writes:

> Again, I also spotted this and thought it was the source. However,
> changing this causes the migration to fail with 1.10 as there are
> objects in this model which must be applied before
> lava_scheduler_app/0001_initial will complete. e.g. the AuthToken
> object is referred to directly in lava_scheduler_app/0001_initial and
> this is defined by linaro_django_xmlrpc

Yes, I was just attempting to publicly document what is causing the
problem, as opposed to coming up with a solution to the problem.

> I tried a few simplistic edits of those migration files on a test
> instance, the migrations still fail to apply.

I had a feeling simple changes would not work here.

My generally feeling at the moment is:

A. Create simple minimal test case with two Django Apps and a script
that does the minimum required to demonstrate the problem.

B. Create a Django bug report pointing to our test case. They may or may
not accept it as a bug in Django, however it would be good to get their
feedback.

C. My idea for a work around is to write code that will directly update
the Django migration tables to indicate that this migration really has
been applied.
-- 
Brian May 



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Neil Williams
On Fri, 26 May 2017 10:16:41 +0100
Neil Williams  wrote:

> On Fri, 26 May 2017 10:05:58 +0100
> Neil Williams  wrote:
> 
> > On Fri, 26 May 2017 18:56:04 +1000
> > Brian May  wrote:
> >   
> > > Neil Williams  writes:
> > > 
> > > > django.db.migrations.exceptions.InconsistentMigrationHistory:
> > > > Migration lava_scheduler_app.0001_initial is applied before its
> > > > dependency linaro_django_xmlrpc.0001_initial on database
> > > > 'default'.  
> > > 
> > > Ok, I see what is going on now. Untested theory at the moment, but
> > > it fits the symptoms.
> > > 
> > > ./lava_scheduler_app/migrations/0001_initial.py contains:
> > > 
> > > dependencies = [  
> > > ('auth', '0001_initial'),  
> > > migrations.swappable_dependency(settings.AUTH_USER_MODEL),  
> > > ('linaro_django_xmlrpc', '__first__'),  
> > > ('dashboard_app', '__first__'),  
> > > ]  
> > 
> > Again, I also spotted this and thought it was the source. However,
> > changing this causes the migration to fail with 1.10 as there are
> > objects in this model which must be applied before
> > lava_scheduler_app/0001_initial will complete. e.g. the AuthToken
> > object is referred to directly in lava_scheduler_app/0001_initial
> > and this is defined by linaro_django_xmlrpc
> > 
> > I tried a few simplistic edits of those migration files on a test
> > instance, the migrations still fail to apply.
> >   
> > > 
> > > My reading of this is that this means this migration depends on
> > > the first migration from 'linaro_django_xmlrpc' to be already
> > > applied. However on Jessie, 'linaro_django_xmlrpc' has no
> > > migrations. Hence I suspect whatever version of Django that
> > > installed this migration to be buggy, because it didn't check the
> > > dependencies could be satisfied. Or maybe this was considered OK
> > > at the time.
> > 
> > 7/9 Add initial Django migrations
> > 
> > From a25d49c6c96217011fead69b675e281b6e5b8fc5 Mon Sep 17
> > 00:00:00 2001
> > From: Raphaël Hertzog  Date: Tue, 9 Sep
> > 2014 07:20:56 + 
> > 
> > https://git.linaro.org/lava/lava-server.git/commit/?id=9e5595adae2b2ea6fc7e19820356ca1bd098110c
> >   
> > > This migration is already applied when we come to do the new
> > > migrations for Django 1.8 or Django 1.10
> > > 
> > > Django 1.8 doesn't care that the first migration for
> > > 'linaro_django_xmlrpc' hasn't been applied yet. As a result, it
> > > can fake the migration and everyone is happy.
> > > 
> > > Django 1.10 does care. It says the database is broken because the
> > > prerequisite for this migration was never applied. It does this
> > > check before applying any migrations.
> > > 
> > > I don't know for sure what is correct behaviour here.
> > 
> > Changing existing migration files *after* user data has been created
> > is likely to cause data loss.
> >   
> > > However I am
> > > inclined to think maybe this isn't a Django 1.10 fault, because
> > > the migration in Jessie clearly says it depends on a migration
> > > that was never applied - because it doesn't even exist at this
> > > point.
> > 
> > No. This is django making the wrong decision about problems it has
> > previously supported when trying to include bug fixes for other
> > problems. It is a regression in django 1.10.
> >   
> 
> Operations to perform:
>   Synchronize unmigrated apps: linaro_django_xmlrpc,
> django_openid_auth, lava_projects, lava_scheduler_daemon,
> django_tables2, lava_markitup Apply all migrations: sessions, admin,
> dashboard_app, auth, sites, contenttypes, lava_scheduler_app,
> google_analytics Synchronizing apps without migrations: Creating
> tables... Creating table django_openid_auth_nonce Creating table
> django_openid_auth_association Creating table
> django_openid_auth_useropenid Creating table
> linaro_django_xmlrpc_authtoken
> 
> From the installation of 2014.9-1 from jessie.
> 
> At no point did 1.7 require or support linaro_django_xmlrpc
> migrations. (Creating them caused django to fail to apply them.) It's
> not that is does not exist, it is that django 1.7 in jessie does not
> allow it to exist - yet 1.10 requires it and it's code in 1.8 that
> handles the transition.

Of the changes between 1.8 and 1.10, this is the problematic function,
new in 1.10

https://sources.debian.net/src/python-django/1:1.10.7-1/django/db/migrations/loader.py/#L270

This code has been relocated from 1.7 to 1.10, relating to __first__:

https://sources.debian.net/src/python-django/1:1.10.7-1/django/db/migrations/loader.py/#L166

I'm wondering if once a migration has been seen by
add_internal_dependencies as __first__, that migration should not be
checked again in check_consistent_history.

-- 


Neil Williams
=
http://www.linux.codehelp.co.uk/



pgpl_DFgYEMnI.pgp
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Neil Williams
On Fri, 26 May 2017 18:56:04 +1000
Brian May  wrote:

> Neil Williams  writes:
> 
> > django.db.migrations.exceptions.InconsistentMigrationHistory:
> > Migration lava_scheduler_app.0001_initial is applied before its
> > dependency linaro_django_xmlrpc.0001_initial on database
> > 'default'.  
> 
> Ok, I see what is going on now. Untested theory at the moment, but it
> fits the symptoms.
> 
> ./lava_scheduler_app/migrations/0001_initial.py contains:
> 
> dependencies = [  
> ('auth', '0001_initial'),  
> migrations.swappable_dependency(settings.AUTH_USER_MODEL),  
> ('linaro_django_xmlrpc', '__first__'),  
> ('dashboard_app', '__first__'),  
> ]  

Again, I also spotted this and thought it was the source. However,
changing this causes the migration to fail with 1.10 as there are
objects in this model which must be applied before
lava_scheduler_app/0001_initial will complete. e.g. the AuthToken
object is referred to directly in lava_scheduler_app/0001_initial and
this is defined by linaro_django_xmlrpc

I tried a few simplistic edits of those migration files on a test
instance, the migrations still fail to apply.

> 
> My reading of this is that this means this migration depends on the
> first migration from 'linaro_django_xmlrpc' to be already
> applied. However on Jessie, 'linaro_django_xmlrpc' has no
> migrations. Hence I suspect whatever version of Django that installed
> this migration to be buggy, because it didn't check the dependencies
> could be satisfied. Or maybe this was considered OK at the time.

7/9 Add initial Django migrations

From a25d49c6c96217011fead69b675e281b6e5b8fc5 Mon Sep 17 00:00:00
2001
From: Raphaël Hertzog  Date: Tue, 9 Sep
2014 07:20:56 + 

https://git.linaro.org/lava/lava-server.git/commit/?id=9e5595adae2b2ea6fc7e19820356ca1bd098110c

> This migration is already applied when we come to do the new
> migrations for Django 1.8 or Django 1.10
> 
> Django 1.8 doesn't care that the first migration for
> 'linaro_django_xmlrpc' hasn't been applied yet. As a result, it can
> fake the migration and everyone is happy.
> 
> Django 1.10 does care. It says the database is broken because the
> prerequisite for this migration was never applied. It does this check
> before applying any migrations.
> 
> I don't know for sure what is correct behaviour here.

Changing existing migration files *after* user data has been created is
likely to cause data loss.

> However I am
> inclined to think maybe this isn't a Django 1.10 fault, because the
> migration in Jessie clearly says it depends on a migration that was
> never applied - because it doesn't even exist at this point.

No. This is django making the wrong decision about problems it has
previously supported when trying to include bug fixes for other
problems. It is a regression in django 1.10.

-- 


Neil Williams
=
http://www.linux.codehelp.co.uk/



pgpNiC3J3VJ8h.pgp
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Brian May
Neil Williams  writes:

> django.db.migrations.exceptions.InconsistentMigrationHistory: Migration
> lava_scheduler_app.0001_initial is applied before its dependency
> linaro_django_xmlrpc.0001_initial on database 'default'.

Ok, I see what is going on now. Untested theory at the moment, but it
fits the symptoms.

./lava_scheduler_app/migrations/0001_initial.py contains:

dependencies = [  
('auth', '0001_initial'),  
migrations.swappable_dependency(settings.AUTH_USER_MODEL),  
('linaro_django_xmlrpc', '__first__'),  
('dashboard_app', '__first__'),  
]  

My reading of this is that this means this migration depends on the
first migration from 'linaro_django_xmlrpc' to be already
applied. However on Jessie, 'linaro_django_xmlrpc' has no
migrations. Hence I suspect whatever version of Django that installed
this migration to be buggy, because it didn't check the dependencies
could be satisfied. Or maybe this was considered OK at the time.

This migration is already applied when we come to do the new migrations
for Django 1.8 or Django 1.10

Django 1.8 doesn't care that the first migration for
'linaro_django_xmlrpc' hasn't been applied yet. As a result, it can fake
the migration and everyone is happy.

Django 1.10 does care. It says the database is broken because the
prerequisite for this migration was never applied. It does this check
before applying any migrations.

I don't know for sure what is correct behaviour here. However I am
inclined to think maybe this isn't a Django 1.10 fault, because the
migration in Jessie clearly says it depends on a migration that was
never applied - because it doesn't even exist at this point.
-- 
Brian May 



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Neil Williams
On Fri, 26 May 2017 18:20:07 +1000
Brian May  wrote:

> Neil Williams  writes:
> 
> >   Applying linaro_django_xmlrpc.0001_initial... FAKED  
> 
> I am speculating this line might be very relevant. Taken from the
> Django 1.8 migration.
> 
> Looks like this never had a migration before, so now needs to be
> faked. I wonder if this is what is triggering the Django 1.10 bug.
> 
> To the best of my knowledge this should still be supported.
> 
> If I am right, then it should be possible to work around this by
> manually applying the migration in fake mode.

My first thought as well, but once the bug has triggered, django 1.10
refuses to even fake migrations, either linaro_django_xmlrpc or
lava_scheduler_app or any of the other apps in lava-server.

1.8 proceeds happily and as long as the user doesn't tinker too much
with trying to force migrations, downgrading to 1.8 does fix the
problem. Get the attempted fix wrong and the migrations fail to the
point where the database has to be restored from backup. (So if you're
testing this locally, snapshot the VM when you get to the point of
django 1.10 installed with lava-server 2016.12-1 partially installed):

iF  lava-server   2016.12-1   all
ii  python-django 1:1.10.7-1  all

I've been thinking it's related to these fixes in 1.10:
https://code.djangoproject.com/ticket/22325
https://code.djangoproject.com/ticket/27004

(as described in python-django-1.10.7/docs/releases/1.10.1.txt)

This is what happens with initial attempts to use --fake

neil@jessie:~$ sudo lava-server manage migrate linaro_django_xmlrpc 
--fake-initial 
...

django.db.migrations.exceptions.InconsistentMigrationHistory: Migration
lava_scheduler_app.0001_initial is applied before its dependency
linaro_django_xmlrpc.0001_initial on database 'default'.


neil@jessie:~$ sudo lava-server manage migrate linaro_django_xmlrpc 
--fake-initial
.
django.db.migrations.exceptions.InconsistentMigrationHistory: Migration
lava_scheduler_app.0001_initial is applied before its dependency
linaro_django_xmlrpc.0001_initial on database 'default'.


neil@jessie:~$ sudo lava-server manage migrate linaro_django_xmlrpc

django.db.migrations.exceptions.InconsistentMigrationHistory: Migration
lava_scheduler_app.0001_initial is applied before its dependency
linaro_django_xmlrpc.0001_initial on database 'default'.

neil@jessie:~$ sudo lava-server manage migrate lava_scheduler_app --fake-initial
.
django.db.migrations.exceptions.InconsistentMigrationHistory: Migration
lava_scheduler_app.0001_initial is applied before its dependency
linaro_django_xmlrpc.0001_initial on database 'default'.

neil@jessie:~$ sudo lava-server manage migrate lava_scheduler_app --fake

django.db.migrations.exceptions.InconsistentMigrationHistory: Migration
lava_scheduler_app.0001_initial is applied before its dependency
linaro_django_xmlrpc.0001_initial on database 'default'.



-- 


Neil Williams
=
http://www.linux.codehelp.co.uk/



pgpwkTDaZUvmm.pgp
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Brian May
Neil Williams  writes:

>   Applying linaro_django_xmlrpc.0001_initial... FAKED

I am speculating this line might be very relevant. Taken from the Django
1.8 migration.

Looks like this never had a migration before, so now needs to be
faked. I wonder if this is what is triggering the Django 1.10 bug.

To the best of my knowledge this should still be supported.

If I am right, then it should be possible to work around this by
manually applying the migration in fake mode.
-- 
Brian May 



Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Neil Williams
On Fri, 26 May 2017 18:05:08 +1000
Brian May  wrote:

> Neil Williams  writes:
> 
> > Upgrading directly to Stretch:  
> 
> Just to clarify: Was this upgrading the entire system to stretch, or
> just the relevant packages?

Starting point was 8.8 jessie, installed postgresql then installed
lava-server.

I added a stretch apt source.list, apt update, apt upgrade (which
brings in python-django 1.10) and then dist-upgrade (which brings in
the lava-server update).

So it was a full upgrade of the entire system - a minimal Debian
installation in a VM with apache, SSH & standard tasks only.


-- 


Neil Williams
=
http://www.linux.codehelp.co.uk/



pgp6aIM7vCVLO.pgp
Description: OpenPGP digital signature


Bug#863267: [Python-modules-team] Bug#863267: Miscalculates MigrationHistory dependencies between multiple django apps - regression from 1.8

2017-05-26 Thread Brian May
Neil Williams  writes:

> Upgrading directly to Stretch:

Just to clarify: Was this upgrading the entire system to stretch, or
just the relevant packages?
-- 
Brian May