Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  Aymeric Augustin  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  dev
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by Michael Manfre):

 Another use case for `class_prepared` is allowing an abstract Model to
 define indexes that 1) require a name (e.g. has conditions) where the
 inheriting class name is too long for the index 30 character limit, or 2)
 should be inherited even if the subclass defines its own indexes.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/010701856e711319-6ccddabb-a57d-443b-b93e-2e18fae87922-00%40eu-central-1.amazonses.com.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  Aymeric Augustin  |Owner:  (none)
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by Gurpreet Singh):

 * owner:  Gurpreet Singh => (none)
 * status:  assigned => new


Comment:

 Replying to [comment:8 Alex Hill]:
 > Alright, there is at least one gap in functionality: using
 `class_prepared`, you can fiddle with models in ways that propagate to
 their subclasses. In `AppConfig.ready()`, since all the `ModelBase`
 inheritance magic has already happened, it's too late to do anything like
 that.

 I shouldn't have chosen to work on this as my first ticket then it seems.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.f3c02bd2c8e3583773d4a2d9723537bc%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
-+-
 Reporter:  Aymeric Augustin |Owner:  Gurpreet
 Type:   |  Singh
  Cleanup/optimization   |   Status:  assigned
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Gurpreet Singh):

 * owner:  nobody => Gurpreet Singh
 * status:  new => assigned


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.3deac6856449f58f732bfbd042eed4c6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by AlexHill):

 Alright, there is at least one gap in functionality: using
 `class_prepared`, you can fiddle with models in ways that propagate to
 their subclasses. In `AppConfig.ready()`, since all the `ModelBase`
 inheritance magic has already happened, it's too late to do anything like
 that.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.3145976a076172cb02afaea2a5d5968f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by AlexHill):

 Yep `class_prepared` is no longer used anywhere in Django other than tests
 AFAIK. Unless someone can outline a use that can't be implemented with
 `AppConfig.ready()`, I agree with deprecating it because
 * it's unlike other signals in that the trigger is generally expected to
 happen only once per sender
 * it's hard to make sure it's registered in the right place

 Mezzanine uses `class_prepared` to add fields to models at boot time,
 which sounds nasty but works surprisingly well. Until Django has a
 sanctioned method of swapping models besides `auth.User`, we'll continue
 to support this. I'll try to refactor it to use `AppConfig.ready()` and
 see how that goes.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.8f4f9ff54b0b38fb528b657b0709be4e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by timgraham):

 The `class_prepared` usage mentioned in my previous comment is removed as
 part of #26421 so it looks like this should be doable after that's merged.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.a7c2fc67dda91b39a7ef17ce5c9c973c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by timgraham):

 There's also a
 
[https://github.com/django/django/blob/27c839e0fce99254ad61322bb827a821f832e840/django/db/models/signals.py#L15-L18
 usage in ModelSignal]. It's not immediately obvious to me how/if this can
 be refactored to use the app registry instead.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.0e851f1d025d13569dc6fd33cdc46864%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by AlexHill):

 Replying to [comment:2 carljm]:
 > For reference, the only place Django internally uses `class_prepared` is
 to call `do_pending_lookups` to handle unresolved relations to the just-
 loaded model. It certainly seems that could/should be handled by the app
 registry instead...

 Hi Carl, I've opened ticket #24215 and
 [https://github.com/django/django/pull/3984/files written a patch] to do
 just this - feedback welcome.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.2025c27f28c0c22375600db0850baf71%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by aaugustin):

 Follow the link in the description for the story of the last time you sent
 me down this rabbit hole :-)

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.f9267d2c642b017651a13920901aaabb%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by carljm):

 For reference, the only place Django internally uses `class_prepared` is
 to call `do_pending_lookups` to handle unresolved relations to the just-
 loaded model. It certainly seems that could/should be handled by the app
 registry instead...

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.f639e43786b9a37d5c6a737058752c0c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
--+
 Reporter:  aaugustin |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Core (Other)  |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by carljm):

 * stage:  Unreviewed => Accepted


Comment:

 I agree. ISTM that since app-loading, Django shouldn't even need
 `class_prepared` internally anymore - but I realize there are dragons in
 that code.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.d2802f44df1043a4e5a3b70cc5ffccbe%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #24313: Deprecate the class_prepared signal

[Django]

#24313: Deprecate the class_prepared signal
+
   Reporter:  aaugustin |  Owner:  nobody
   Type:  Cleanup/optimization  | Status:  new
  Component:  Core (Other)  |Version:  master
   Severity:  Normal|   Keywords:
   Triage Stage:  Unreviewed|  Has patch:  0
Needs documentation:  0 |Needs tests:  0
Patch needs improvement:  0 |  Easy pickings:  0
  UI/UX:  0 |
+
 Since 1.7, whatever can be done in a `class_prepared` listener is probably
 better done by inspecting the app registry in `AppConfig.ready()`.

 As the documentation notes, reacting to `class_prepared` is impractical
 because the app-loading and meta-refactor APIs don't work yet.

 Even if we keep the signal — I once tried to refactor this area and it
 [https://code.djangoproject.com/ticket/21719#comment:13 went downhill
 quickly] — I think we should remove it from the public API.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/052.9d57d351819f62ddfac2bf69fd393b69%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.