Re: Infinite loop in migration code

2014-11-27 Thread Markus Holtermann
Hey Luke,

I cannot reproduce your problem on neither master nor stable/1.7.x. When I 
run "python manage.py makemigrations camps" the migrations for the 
"officers" app are also generated (as I expect it). "testapp" is your 
"camps" app, "otherapp" is your "officers" app: 
https://gist.github.com/MarkusH/d84618db929fd6fcdb9f

The dependencies look fine to me, and migrating works like a charm. I also 
tried both the string representation 'auth.User' and importing d.c.a.m.User 
and using the class reference in the FK fields, but that doesn't make any 
difference.

If you can reproduce the issue, could you please push a sample project to 
GitHub. I'm happy to look into it then.

/Markus

On Thursday, November 27, 2014 5:20:52 PM UTC+1, Luke Plant wrote:
>
> Hi Markus, 
>
>
> It was basically this: 
>
> == app camps == 
>
> class Camp: 
>
>   invited_officers = M2M(auth.User, through='officers.Invitation') 
>
>
> == app officers == 
>
> class Invitation: 
>   timestamp = models.DateTimeField(default=datetime.now) 
>   camp = FK(camps.Camp) 
>   user = FK(auth.User) 
>
>
> = 
>
> I ran makemigrations for 'camps', then 'officers', and the generated 
> migrations ended up depending on each other. 
>
> Hopefully that's enough, let me know if that doesn't reproduce it. 
> Sorry, don't have time to put it together more formally. 
>
> Thanks, 
>
> Luke 
>
>
> On 26/11/14 13:08, Markus Holtermann wrote: 
> > Can you open a ticket with your models so the issue doesn't get lost. 
> > I'm happy to work on it. 
> > 
> > Although it's somewhat uncommon, people normally have the through model 
> > in the app that has the m2m field (why don't you define it on the other 
> > model?) this is still something that shouldn't happen. 
> > 
> > /Markus 
> > 
> > On Wednesday, November 26, 2014 8:54:55 AM UTC+1, Luke Plant wrote: 
> > 
> > On 25/11/14 16:23, Markus Holtermann wrote: 
> > > Hey Luke, 
> > > 
> > > It would be interesting to see why A.1 and B.1 depend on each 
> > other. If 
> > > there are e.g. FK constraints pointing to models in the other app 
> the 
> > > autodetector should end up with e.g. A.1 <-- B.1 <-- B.2 <-- A.2 
> (or 
> > > optimized A.1 <-- B.1 <-- A.2), in which case you wouldn't end up 
> > with a 
> > > cycle. C.1 would then depend on B.2 (B.1 respectively in the 
> > optimized 
> > > graph). 
> > 
> > I didn't realise the autodetector could handle that. Looking more 
> > closely, it looks like I have more of an edge case: 
> > 
> > App B has a model with a FK to a model in app A 
> > 
> > App A has a model with a ManyToMany field 'through' a model in app 
> B. 
> > (It's actually added that way for the sake of the admin for the 
> models 
> > in app A). 
> > 
> > So it isn't the straight-forward A has FK to B. It might not be 
> worth 
> > fixing the autodetector for this, as fixing the migrations is 
> > relatively 
> > easy. But I think fixing the infinite loop is another matter, and 
> I'll 
> > go ahead and backport that. 
> > 
> > Thanks for the input, 
> > 
> > Luke 
> > 
> > 
> > -- 
> > "We may not return the affection of those who like us, but we 
> > always respect their good judgement." -- Libbie Fudim 
> > 
> > Luke Plant || http://lukeplant.me.uk/ 
>
>
> -- 
> "In your presence there is fullness of joy; at your right hand are 
> pleasures forevermore" Psalm 16:11 
>
> Luke Plant || http://lukeplant.me.uk/ 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/83263507-d328-4b6e-95d6-e8a2e9d557f6%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Infinite loop in migration code

2014-11-27 Thread Luke Plant
Hi Markus,


It was basically this:

== app camps ==

class Camp:

  invited_officers = M2M(auth.User, through='officers.Invitation')


== app officers ==

class Invitation:
  timestamp = models.DateTimeField(default=datetime.now)
  camp = FK(camps.Camp)
  user = FK(auth.User)


=

I ran makemigrations for 'camps', then 'officers', and the generated
migrations ended up depending on each other.

Hopefully that's enough, let me know if that doesn't reproduce it.
Sorry, don't have time to put it together more formally.

Thanks,

Luke


On 26/11/14 13:08, Markus Holtermann wrote:
> Can you open a ticket with your models so the issue doesn't get lost.
> I'm happy to work on it.
> 
> Although it's somewhat uncommon, people normally have the through model
> in the app that has the m2m field (why don't you define it on the other
> model?) this is still something that shouldn't happen.
> 
> /Markus
> 
> On Wednesday, November 26, 2014 8:54:55 AM UTC+1, Luke Plant wrote:
> 
> On 25/11/14 16:23, Markus Holtermann wrote:
> > Hey Luke,
> >
> > It would be interesting to see why A.1 and B.1 depend on each
> other. If
> > there are e.g. FK constraints pointing to models in the other app the
> > autodetector should end up with e.g. A.1 <-- B.1 <-- B.2 <-- A.2 (or
> > optimized A.1 <-- B.1 <-- A.2), in which case you wouldn't end up
> with a
> > cycle. C.1 would then depend on B.2 (B.1 respectively in the
> optimized
> > graph).
> 
> I didn't realise the autodetector could handle that. Looking more
> closely, it looks like I have more of an edge case:
> 
> App B has a model with a FK to a model in app A
> 
> App A has a model with a ManyToMany field 'through' a model in app B.
> (It's actually added that way for the sake of the admin for the models
> in app A).
> 
> So it isn't the straight-forward A has FK to B. It might not be worth
> fixing the autodetector for this, as fixing the migrations is
> relatively
> easy. But I think fixing the infinite loop is another matter, and I'll
> go ahead and backport that.
> 
> Thanks for the input,
> 
> Luke
> 
> 
> -- 
> "We may not return the affection of those who like us, but we
> always respect their good judgement." -- Libbie Fudim
> 
> Luke Plant || http://lukeplant.me.uk/
> 
> -- 
> You received this message because you are subscribed to the Google
> Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to django-developers+unsubscr...@googlegroups.com
> .
> To post to this group, send email to django-developers@googlegroups.com
> .
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/f2924e77-7604-4632-a7c1-1920a3bfb4d0%40googlegroups.com
> .
> For more options, visit https://groups.google.com/d/optout.


-- 
"In your presence there is fullness of joy; at your right hand are
pleasures forevermore" Psalm 16:11

Luke Plant || http://lukeplant.me.uk/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/54774F2E.2020306%40cantab.net.
For more options, visit https://groups.google.com/d/optout.


Re: Infinite loop in migration code

2014-11-26 Thread Markus Holtermann
Can you open a ticket with your models so the issue doesn't get lost. I'm 
happy to work on it.

Although it's somewhat uncommon, people normally have the through model in 
the app that has the m2m field (why don't you define it on the other 
model?) this is still something that shouldn't happen.

/Markus

On Wednesday, November 26, 2014 8:54:55 AM UTC+1, Luke Plant wrote:
>
> On 25/11/14 16:23, Markus Holtermann wrote: 
> > Hey Luke, 
> > 
> > It would be interesting to see why A.1 and B.1 depend on each other. If 
> > there are e.g. FK constraints pointing to models in the other app the 
> > autodetector should end up with e.g. A.1 <-- B.1 <-- B.2 <-- A.2 (or 
> > optimized A.1 <-- B.1 <-- A.2), in which case you wouldn't end up with a 
> > cycle. C.1 would then depend on B.2 (B.1 respectively in the optimized 
> > graph). 
>
> I didn't realise the autodetector could handle that. Looking more 
> closely, it looks like I have more of an edge case: 
>
> App B has a model with a FK to a model in app A 
>
> App A has a model with a ManyToMany field 'through' a model in app B. 
> (It's actually added that way for the sake of the admin for the models 
> in app A). 
>
> So it isn't the straight-forward A has FK to B. It might not be worth 
> fixing the autodetector for this, as fixing the migrations is relatively 
> easy. But I think fixing the infinite loop is another matter, and I'll 
> go ahead and backport that. 
>
> Thanks for the input, 
>
> Luke 
>
>
> -- 
> "We may not return the affection of those who like us, but we 
> always respect their good judgement." -- Libbie Fudim 
>
> Luke Plant || http://lukeplant.me.uk/ 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/f2924e77-7604-4632-a7c1-1920a3bfb4d0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Infinite loop in migration code

2014-11-25 Thread Luke Plant
On 25/11/14 16:23, Markus Holtermann wrote:
> Hey Luke,
> 
> It would be interesting to see why A.1 and B.1 depend on each other. If
> there are e.g. FK constraints pointing to models in the other app the
> autodetector should end up with e.g. A.1 <-- B.1 <-- B.2 <-- A.2 (or
> optimized A.1 <-- B.1 <-- A.2), in which case you wouldn't end up with a
> cycle. C.1 would then depend on B.2 (B.1 respectively in the optimized
> graph).

I didn't realise the autodetector could handle that. Looking more
closely, it looks like I have more of an edge case:

App B has a model with a FK to a model in app A

App A has a model with a ManyToMany field 'through' a model in app B.
(It's actually added that way for the sake of the admin for the models
in app A).

So it isn't the straight-forward A has FK to B. It might not be worth
fixing the autodetector for this, as fixing the migrations is relatively
easy. But I think fixing the infinite loop is another matter, and I'll
go ahead and backport that.

Thanks for the input,

Luke


-- 
"We may not return the affection of those who like us, but we
always respect their good judgement." -- Libbie Fudim

Luke Plant || http://lukeplant.me.uk/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5475873E.3010805%40cantab.net.
For more options, visit https://groups.google.com/d/optout.


Re: Infinite loop in migration code

2014-11-25 Thread Andrew Godwin
Hi Luke,

That was already a fix for infinite looping on the previous iteration that
I committed at the DUTH sprints, but your fix looks more understandable and
cleaner, so I say commit it for sure.

As for backporting - I think we should, as this is potentially a crash bug
(though not a data loss bug) and it's a pretty small thing to backport
anyway.

Andrew


On Tue, Nov 25, 2014 at 8:23 AM, Markus Holtermann  wrote:

> Hey Luke,
>
> It would be interesting to see why A.1 and B.1 depend on each other. If
> there are e.g. FK constraints pointing to models in the other app the
> autodetector should end up with e.g. A.1 <-- B.1 <-- B.2 <-- A.2 (or
> optimized A.1 <-- B.1 <-- A.2), in which case you wouldn't end up with a
> cycle. C.1 would then depend on B.2 (B.1 respectively in the optimized
> graph).
>
> /Markus
>
>
> On Tuesday, November 25, 2014 4:49:22 PM UTC+1, Luke Plant wrote:
>>
>>
>> I came across a bug with an infinite loop in migration dependency
>> searching code. This is fixed here:
>>
>> https://github.com/django/django/commit/ff3d746e8d8e8fbe6de287bd0f4c3a
>> 9fa23c18dc
>>
>> (another person reviewing it would be good, though I think it is
>> correct).
>>
>> My question is, should we backport this to 1.7.x? For me, the bug
>> manifested itself with migrations that were automatically created by
>> Django itself, in a project with apps A, B, and C:
>>
>> App B depends on app A
>> App A depends on app B (it didn't initially, but does now)
>> App C depends on one/both of them.
>>
>> After doing makemigrations for A and B, makemigrations for C then went
>> into an infinite loop.
>>
>> So this is not a really obscure case, and could affect a fair number of
>> people attempting to upgrade to Django 1.7, as I was.
>>
>> Regards,
>>
>> Luke
>>
>>
>> --
>> "We may not return the affection of those who like us, but we
>> always respect their good judgement." -- Libbie Fudim
>>
>> Luke Plant || http://lukeplant.me.uk/
>>
>  --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/5e547550-1002-4a6b-82f0-a8dc7e3ed2b0%40googlegroups.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAFwN1uq3zkayXL8%3D1QtGpY8hZcYwC64VRXL%2BEXPC6c9nH%3Dawwg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Infinite loop in migration code

2014-11-25 Thread Markus Holtermann
Hey Luke,

It would be interesting to see why A.1 and B.1 depend on each other. If 
there are e.g. FK constraints pointing to models in the other app the 
autodetector should end up with e.g. A.1 <-- B.1 <-- B.2 <-- A.2 (or 
optimized A.1 <-- B.1 <-- A.2), in which case you wouldn't end up with a 
cycle. C.1 would then depend on B.2 (B.1 respectively in the optimized 
graph).

/Markus

On Tuesday, November 25, 2014 4:49:22 PM UTC+1, Luke Plant wrote:
>
>
> I came across a bug with an infinite loop in migration dependency 
> searching code. This is fixed here: 
>
>
> https://github.com/django/django/commit/ff3d746e8d8e8fbe6de287bd0f4c3a9fa23c18dc
>  
>
> (another person reviewing it would be good, though I think it is correct). 
>
> My question is, should we backport this to 1.7.x? For me, the bug 
> manifested itself with migrations that were automatically created by 
> Django itself, in a project with apps A, B, and C: 
>
> App B depends on app A 
> App A depends on app B (it didn't initially, but does now) 
> App C depends on one/both of them. 
>
> After doing makemigrations for A and B, makemigrations for C then went 
> into an infinite loop. 
>
> So this is not a really obscure case, and could affect a fair number of 
> people attempting to upgrade to Django 1.7, as I was. 
>
> Regards, 
>
> Luke 
>
>
> -- 
> "We may not return the affection of those who like us, but we 
> always respect their good judgement." -- Libbie Fudim 
>
> Luke Plant || http://lukeplant.me.uk/ 
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5e547550-1002-4a6b-82f0-a8dc7e3ed2b0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Infinite loop in migration code

2014-11-25 Thread Luke Plant

I came across a bug with an infinite loop in migration dependency
searching code. This is fixed here:

https://github.com/django/django/commit/ff3d746e8d8e8fbe6de287bd0f4c3a9fa23c18dc

(another person reviewing it would be good, though I think it is correct).

My question is, should we backport this to 1.7.x? For me, the bug
manifested itself with migrations that were automatically created by
Django itself, in a project with apps A, B, and C:

App B depends on app A
App A depends on app B (it didn't initially, but does now)
App C depends on one/both of them.

After doing makemigrations for A and B, makemigrations for C then went
into an infinite loop.

So this is not a really obscure case, and could affect a fair number of
people attempting to upgrade to Django 1.7, as I was.

Regards,

Luke


-- 
"We may not return the affection of those who like us, but we
always respect their good judgement." -- Libbie Fudim

Luke Plant || http://lukeplant.me.uk/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5474A4F1.20208%40cantab.net.
For more options, visit https://groups.google.com/d/optout.