Re: extra files in startproject

[Florian Apolloner]

On Friday, April 13, 2012 6:49:32 AM UTC+2, Alex Ogier wrote:
>
> I have seen setup.py's that use remove_tree as part of a "clean" command
> to allow someone to run "setup.py clean && setup.py install" to obtain
> a pristine distribution idempotently, which I think is a good idea.
>
No, they should work on fixing distutils instead of creating solutions 
which probably could break even worse.
 

> The alternative is to have everyone remember to "rm -rf" their
> site-packages django every time they run setup.py install which is a
> bit unsavory in my opinion.
>
Or just tell them to use either pip even for development installs or just 
set their PYTHONPATH.
 

> If someone has managed to get extra files in their site-packages,
> because at any point they followed a tutorial on how to build from
> source, then their django installation is basically caput until they
> manually "rm -rf" a deep library path. One option is to document this
> and explain what to do
>
You made me lol, that approach is documented in the install guide: 
https://docs.djangoproject.com/en/dev/intro/install/#remove-any-old-versions-of-django
 
-- If people would actually read the docs this issue wouldn't exist. FWIW 
the docs also mention to symlink a dev checkout and don't tell you to run 
setup.py
 

> That would mean listing somewhere the files from
> django/conf/project_template/ that should be included, which isn't
> very DRY, but is the only 100% solution I think.
>
Given that the documentation shows how to do it properly I don't see any 
point. Especially since this problem isn't related to the project_template 
alone -- that's just where it's most visible.

 

> So, that should give you some idea of the perils of not cleaning your
> output directories (or in this case, input directory).
>
We are aware of those, and fwiw: If you use git and switch branches it's up 
to you to know how python works and how git clean works, or do you want to 
suggest that django should rm al pyc files on startup?!
 

> My recommendation is to make "setup.py clean" do everything possible
> to ensure idempotent installation across any version, document that,
> and call it a day. 
>
What's wrong with the current documented approach? (Aside from the fact 
that people don't read it, but then again they won't read the setup.py 
clean either). 

Regards,
Florian

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/SbdWA7plRx4J.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Alex Ogier]

On Thu, Apr 12, 2012 at 11:56 PM, Ben Finney  wrote:
>
> Alex Ogier  writes:
>
> > That seems like too much to ask. "setup.py install" should Just
> > Work(tm),
>
> In the absence of a proper package management system (as implemented in
> operating systems that solved this problem decades ago), you can't
> expect it to Just Work. Parallel installation of multiple versions,
> detection of previous versions, upgrade and uninstallation, dependency
> management – these are problems solved by a package manager, and
> ‘setup.py’ doesn't play well with them.
>
> Python's ‘setup.py’ can't be expected to do the job of a package
> manager, especially not in parallel to the OS which often has its own
> package manager.


The problem is that not everyone uses package managers. A reasonable
way to track django trunk for example is to periodically pull and run
"setup.py install" which is in most cases approximately idempotent. I
have seen setup.py's that use remove_tree as part of a "clean" command
to allow someone to run "setup.py clean && setup.py install" to obtain
a pristine distribution idempotently, which I think is a good idea.
The alternative is to have everyone remember to "rm -rf" their
site-packages django every time they run setup.py install which is a
bit unsavory in my opinion.

If someone has managed to get extra files in their site-packages,
because at any point they followed a tutorial on how to build from
source, then their django installation is basically caput until they
manually "rm -rf" a deep library path. One option is to document this
and explain what to do, but this really isn't very discoverable
because telling people that when django is broken, running "sudo rm
-rf" in their python site-packages might fix it is rather akin to
someone calling tech support and them asking, "Well, did you turn off
your computer and turn it on again?" It might fix the problem, but
it's invasive and time-consuming and it shouldn't be necessary.

The "real" solution to this problem is to stop treating the existence
of files as implicit indication of their inclusion in Django. That
would mean listing somewhere the files from
django/conf/project_template/ that should be included, which isn't
very DRY, but is the only 100% solution I think.

I was bit by this sort of thing a few weeks ago. I had just removed
the simplejson/__init__.py{c,} module and related code and installed a
shim at simplejson.py, all in a feature branch in git. Imagine my
surprise, upon switching to the feature branch several days later, I
found that django was using the old version. The reason was that when
I switched back to the master branch, the simplejson/__init__.pyc was
recompiled and upon checkout out the feature branch git happily
ignored all the .pyc files in my tree. Python saw them and assumed I
had a compiled version of the module and continued using them. So,
that should give you some idea of the perils of not cleaning your
output directories (or in this case, input directory).

My recommendation is to make "setup.py clean" do everything possible
to ensure idempotent installation across any version, document that,
and call it a day. Yes, Django can't make up for people who circumvent
their package manager, but we can make it a lot easier to fix than
sending newbies off into their system libraries armed with superuser
permissions and "rm -rf".

Best,
Alex Ogier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Ben Finney]

Alex Ogier  writes:

> That seems like too much to ask. "setup.py install" should Just
> Work(tm),

In the absence of a proper package management system (as implemented in
operating systems that solved this problem decades ago), you can't
expect it to Just Work. Parallel installation of multiple versions,
detection of previous versions, upgrade and uninstallation, dependency
management – these are problems solved by a package manager, and
‘setup.py’ doesn't play well with them.

Python's ‘setup.py’ can't be expected to do the job of a package
manager, especially not in parallel to the OS which often has its own
package manager.

-- 
 \   “For fast acting relief, try slowing down.” —Jane Wagner, via |
  `\   Lily Tomlin |
_o__)  |
Ben Finney

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: django db library doesn't handle quoted table/field names

[Ramiro Morales]

On Thu, Apr 12, 2012 at 9:14 PM, Craig Lucas  wrote:
> i actually didnt try the db_column attribute because I read about quoting it
> before I started writing the code. But that wont solve the schema problem.
>  It will still try to create the index and fk constraints with the dots and
> quotes around the table name.
>
> i believe it is required to quote the schema and table name in the db_table
> property as '"dim"."Application"', if you left it as 'dim.Application' it
> would create the table as 'dim.application' and if it quoted it it would
> still be wrong as "dim.Application"...that will fail also
>
> On the schema question, yes adding a db_schema property and all the
> supporting code sounds like it would fit well...you can create fk's to
> tables in diff schemas.

This is ticket [1]#6148. There is some recent renewed energy and
know how being put in addind that feature. Maybe you can help by
testing the  proposed changes with your use case and giving
feedback?

Regards,

-- 
Ramiro Morales

1. https://code.djangoproject.com/ticket/6148

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Luciano Pacheco]

On Fri, Apr 13, 2012 at 8:30 AM, Carl Meyer  wrote:

> Hi Alex,
>
> On 04/12/2012 04:23 PM, Alex Ogier wrote:
> > On Apr 12, 2012 6:16 PM, "Carl Meyer"  > > wrote:
>
[...]

>  I still think the right solution is to encourage (via the documentation)
> installation practices that work reliably, not to try to apply piecemeal
> workarounds to specific breakages caused by installation practices that
> don't work reliably (and still won't after the piecemeal workaround).


Please add a note how to detect Django version and path:

>>> import django
>>> django.VERSION
(1, 3, 0, 'final', 0)
>>> django.__file__
'/home/lucianopacheco/src/tmp_py/local/lib/python2.7/site-packages/django/__init__.pyc'


So, it will be much more clear to people not familiar with python
installation structure to understand what "site-packages" means.

[],
-- 
Luciano Pacheco
blog.lucmult.com.br

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: django db library doesn't handle quoted table/field names

[Craig Lucas]

i actually didnt try the db_column attribute because I read about quoting
it before I started writing the code. But that wont solve the schema
problem.  It will still try to create the index and fk constraints with the
dots and quotes around the table name.

i believe it is required to quote the schema and table name in the db_table
property as '"dim"."Application"', if you left it as 'dim.Application' it
would create the table as 'dim.application' and if it quoted it it would
still be wrong as "dim.Application"...that will fail also

On the schema question, yes adding a db_schema property and all the
supporting code sounds like it would fit well...you can create fk's to
tables in diff schemas.

Craig

On Thu, Apr 12, 2012 at 7:25 PM, Jeremy Dunck  wrote:

> On Thu, Apr 12, 2012 at 3:06 PM, Craig Lucas 
> wrote:
> > i have developed a database for a client that uses camel casing in the
> > database. we are now trying to get a models.py file to mimic the
> > database structure and i have run into a few bugs with regards to
> > using quoted table names.
> >
> > The first issue is when you specify a foreign key field it takes the
> > table name + the field name to generate a unique constraint name. This
> > logic does not take into consideration that the field name has quotes
> > around it so it fails when executing the sql to create the foreign key
> > constraint. This wouldnt be so much of a problem if you could specify
> > the name of the foreign key (which I use through the related_name
> > parameter) and the engine would actually use it as the constraint
> > name.
>
> Did the db_column attribute not work for this?  You shouldn't need to
> do literal quotes in the value you pass to db_column - django
> generally quotes field names in all cases.
> https://docs.djangoproject.com/en/1.4/ref/models/fields/#db-column
>
> > The next problem is somewhat similar. The django toolkit tries to
> > automatically create indexes on foreign key fields but doesnt take
> > into consideration that the table has quotes around it OR that the
> > table is in a schema.
> > Note that I override the db_table to force it to be in a schema.
> >
> > In the django code in creation.py:
> >
> > def get_index_sql(index_name, opclass=''):
> >return (style.SQL_KEYWORD('CREATE INDEX') + ' ' +
> >
> >
> style.SQL_TABLE(qn(truncate_name(index_name,self.connection.ops.max_name_length(.replace('"','').replace('.',
> > '_') + ' ' +
> >style.SQL_KEYWORD('ON') + ' ' +
> >style.SQL_TABLE(qn(db_table)) + ' ' +
> >"(%s%s)" % (style.SQL_FIELD(qn(f.column)),
> > opclass) +
> >"%s;" % tablespace_sql)
> >
> > I have added the ".replace('"','').replace('.', '_')" part to format
> > the name properly to account for customized table names.
>
> You're right that there isn't support for schema, though there is for
> db_table, db_column and db_tablespace.  I think db_schema fits fine,
> as long as the semantics make sense - I know schema is a common
> concept, but do different backends have a similar notion?  Can foreign
> keys to schemas be done, for example?
>
> ...
> >   ApplicationKey = models.AutoField(primary_key=True, db_column =
> > '"ApplicationKey"')
>
> For what it's worth, I would have expected this to work:
> ApplicationKey = models.AutoField(primary_key=True, db_column =
> 'ApplicationKey')
>
> And to be more pythonic:
> application_key = models.AutoField(primary_key=True,
> db_column='ApplicationKey')
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: django db library doesn't handle quoted table/field names

[Jeremy Dunck]

On Thu, Apr 12, 2012 at 3:06 PM, Craig Lucas  wrote:
> i have developed a database for a client that uses camel casing in the
> database. we are now trying to get a models.py file to mimic the
> database structure and i have run into a few bugs with regards to
> using quoted table names.
>
> The first issue is when you specify a foreign key field it takes the
> table name + the field name to generate a unique constraint name. This
> logic does not take into consideration that the field name has quotes
> around it so it fails when executing the sql to create the foreign key
> constraint. This wouldnt be so much of a problem if you could specify
> the name of the foreign key (which I use through the related_name
> parameter) and the engine would actually use it as the constraint
> name.

Did the db_column attribute not work for this?  You shouldn't need to
do literal quotes in the value you pass to db_column - django
generally quotes field names in all cases.
https://docs.djangoproject.com/en/1.4/ref/models/fields/#db-column

> The next problem is somewhat similar. The django toolkit tries to
> automatically create indexes on foreign key fields but doesnt take
> into consideration that the table has quotes around it OR that the
> table is in a schema.
> Note that I override the db_table to force it to be in a schema.
>
> In the django code in creation.py:
>
> def get_index_sql(index_name, opclass=''):
>                return (style.SQL_KEYWORD('CREATE INDEX') + ' ' +
>
> style.SQL_TABLE(qn(truncate_name(index_name,self.connection.ops.max_name_length(.replace('"','').replace('.',
> '_') + ' ' +
>                        style.SQL_KEYWORD('ON') + ' ' +
>                        style.SQL_TABLE(qn(db_table)) + ' ' +
>                        "(%s%s)" % (style.SQL_FIELD(qn(f.column)),
> opclass) +
>                        "%s;" % tablespace_sql)
>
> I have added the ".replace('"','').replace('.', '_')" part to format
> the name properly to account for customized table names.

You're right that there isn't support for schema, though there is for
db_table, db_column and db_tablespace.  I think db_schema fits fine,
as long as the semantics make sense - I know schema is a common
concept, but do different backends have a similar notion?  Can foreign
keys to schemas be done, for example?

...
>   ApplicationKey = models.AutoField(primary_key=True, db_column =
> '"ApplicationKey"')

For what it's worth, I would have expected this to work:
ApplicationKey = models.AutoField(primary_key=True, db_column =
'ApplicationKey')

And to be more pythonic:
application_key = models.AutoField(primary_key=True, db_column='ApplicationKey')

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Carl Meyer]

Hi Alex,

On 04/12/2012 04:23 PM, Alex Ogier wrote:
> On Apr 12, 2012 6:16 PM, "Carl Meyer"  > wrote:
>> The correct solution is to warn people away from using installation
>> techniques in ways they were not intended to be used, and that don't
>> work correctly. Repeated use of "setup.py install" without removing the
>> previously-installed version is inherently broken; if we work around one
>> specific case where it breaks, there will be others in the future (there
>> probably already are).
>>
> There are ways to support cleaning directories as part of the 'install'
> command, for example 'distutils.dir_util.remove_tree'. Adding that for
> our specific directory that needs to be clean should work, yes?

The entire django/ subtree should be clean before a new install of
Django, not just the new project template. Startproject is not unique,
just more visibly problematic. Off the top of my head, a stray file in
the built-in management commands directory would also cause an issue -
and a stray .py file anywhere would cause imports to continue to work
(and bring in outdated code) when they ought to fail for that version of
Django.

I'm firmly -1 on any workaround that specifically targets the project
template; it's just encouraging people to continue to use an
installation method that will cause other breakages, perhaps subtler ones.

It's possible that Django's setup.py could manually remove the entire
django/ directory from the target site-packages, but I don't think that
would be a good idea either; it'd be non-standard behavior that would
break the expectations of anyone who's used other setup.py files.

I still think the right solution is to encourage (via the documentation)
installation practices that work reliably, not to try to apply piecemeal
workarounds to specific breakages caused by installation practices that
don't work reliably (and still won't after the piecemeal workaround).

Carl



signature.asc
Description: OpenPGP digital signature

Re: extra files in startproject

[Alex Ogier]

On Apr 12, 2012 6:16 PM, "Carl Meyer"  wrote:
>
> The correct solution is to warn people away from using installation
> techniques in ways they were not intended to be used, and that don't
> work correctly. Repeated use of "setup.py install" without removing the
> previously-installed version is inherently broken; if we work around one
> specific case where it breaks, there will be others in the future (there
> probably already are).
>

There are ways to support cleaning directories as part of the 'install'
command, for example 'distutils.dir_util.remove_tree'. Adding that for our
specific directory that needs to be clean should work, yes?

Best,
Alex Ogier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Carl Meyer]

On 04/12/2012 03:27 PM, Alex Ogier wrote:
> That seems like too much to ask. "setup.py install" should Just
> Work(tm), and if django requires that a certain directory be clean of
> any .py files in order to function properly then "setup.py install"
> should make that happen. The note might still be valuable, because we
> should inform people that untar-ing a version on top of another can
> cause problems like this, but breaking "setup.py install" shouldn't be
> necessary.

Who is breaking or proposing to break "setup.py install"? Distutils
never designed "setup.py install" to support repeated installations into
the same location without removing the old version first. You could
argue that's an issue with distutils, but it's certainly not in scope
for Django to fix. In general, an installed Python package should be
able to assume that there aren't random additional .py files scattered
about in its source tree that aren't part of the source tree for that
version.

The correct solution is to warn people away from using installation
techniques in ways they were not intended to be used, and that don't
work correctly. Repeated use of "setup.py install" without removing the
previously-installed version is inherently broken; if we work around one
specific case where it breaks, there will be others in the future (there
probably already are).

> If none of these are acceptable, then we can always use a different
> directory (django/conf/project_template_new), so that conflicts don't
> happen. It's a bit hacky but it would work.

No thanks.

Carl



signature.asc
Description: OpenPGP digital signature

django db library doesn't handle quoted table/field names

[Craig Lucas]

i have developed a database for a client that uses camel casing in the
database. we are now trying to get a models.py file to mimic the
database structure and i have run into a few bugs with regards to
using quoted table names.

The first issue is when you specify a foreign key field it takes the
table name + the field name to generate a unique constraint name. This
logic does not take into consideration that the field name has quotes
around it so it fails when executing the sql to create the foreign key
constraint. This wouldnt be so much of a problem if you could specify
the name of the foreign key (which I use through the related_name
parameter) and the engine would actually use it as the constraint
name.

The code is located in creation.py in sql_for_pending_references():

final_output.append(style.SQL_KEYWORD('ALTER TABLE') +
' %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES
%s (%s)%s;' %
(qn(r_table), qn(truncate_name(
r_name,
self.connection.ops.max_name_length())),
qn(r_col), qn(table), qn(col),
self.connection.ops.deferrable_sql()))

I have personally changed the code above this to include:

#if the related_name property is set then use it as the foreign key
name, otherwise generate it as before

if f.rel.related_name == '':
r_name = '%s_refs_%s_%s' % (r_col, col,
self._digest(r_table, table))
r_name = r_name.replace('"','')
else:
r_name = f.rel.related_name

This gets rid of the errors creating the constraint.

The next problem is somewhat similar. The django toolkit tries to
automatically create indexes on foreign key fields but doesnt take
into consideration that the table has quotes around it OR that the
table is in a schema.
Note that I override the db_table to force it to be in a schema.

In the django code in creation.py:

def get_index_sql(index_name, opclass=''):
return (style.SQL_KEYWORD('CREATE INDEX') + ' ' +
 
style.SQL_TABLE(qn(truncate_name(index_name,self.connection.ops.max_name_length(.replace('"','').replace('.',
'_') + ' ' +
style.SQL_KEYWORD('ON') + ' ' +
style.SQL_TABLE(qn(db_table)) + ' ' +
"(%s%s)" % (style.SQL_FIELD(qn(f.column)),
opclass) +
"%s;" % tablespace_sql)

I have added the ".replace('"','').replace('.', '_')" part to format
the name properly to account for customized table names.

I am sure there are other areas that have this issue, but havent
encountered them yet. What do you think about including this into the
next version of django?

Here is my PY file:

class Application(models.Model):
   class Meta:
   db_table = '"dim"."Application"'
   ApplicationKey = models.AutoField(primary_key=True, db_column =
'"ApplicationKey"')
   ParentApplicationKey = models.ForeignKey('Application',
related_name = '"fkParentApplicationKey"', db_column =
'"ParentApplicationKey"', to_field = '"ApplicationKey"')
   Name = models.CharField(max_length=256, db_column = '"Name"')
   Description = models.TextField(db_column = '"Description"')
   CPE = models.CharField(max_length=256, db_column = '"CPE"')
   SurrogateKey = models.IntegerField(db_column = '"SurrogateKey"')
   SurrogateParentKey = models.IntegerField(db_column =
'"SurrogateParentKey"')
   DataSourceKey = models.ForeignKey('DataSource', related_name =
'"fkDataSourceKey"', db_column = '"DataSourceKey"')

class DataSource(models.Model):
   class Meta:
   db_table = '"dim"."DataSource"'
   DataSourceKey = models.AutoField(primary_key=True, db_column =
'"DataSourceKey"')
   DataSourceType = models.IntegerField(db_column =
'"DataSourceType"')
   Enabled = models.BooleanField(db_column = '"Enabled"')
   Name = models.CharField(max_length=256, db_column = '"Name"')
   Description = models.CharField(max_length=4000, db_column =
'"Description"')
   HostName = models.CharField(max_length=256, db_column =
'"HostName"')
   UserName = models.CharField(max_length=256, db_column =
'"UserName"')
   Password = models.CharField(max_length=256, db_column =
'"Password"')
   APIVersion = models.CharField(max_length=256, db_column =
'"APIVersion"')

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Carl Meyer]

On 04/12/2012 03:16 PM, Aymeric Augustin wrote:
> This problem occurs at least when you run "setup.py install" before
> and after the commit that introduced the new project layout.
> 
> Some people who had the habit of running "setup.py install" from a
> git clone to keep up-to-date with the development version reported
> the problem. (Just to be 100% clear — this technique doesn't work
> because it doesn't remove .py or .pyc files that are removed from
> Django.) As a result, I added a warning to the docs in r17636.

Ah, this makes sense. Thanks!

> Most likely, installing 1.4 with "setup.py install", as explained in
> our docs [1], has the same result when 1.3 was previously installed
> in the same fashion.

Yes, it would.

> I suggest we add a warning to this section of the docs: "if a
> previous version of Django is installed, go delete it manually in
> your site-packages directory" (unless there's a better method to
> remove a Python package?)

The "better way" would normally be "pip uninstall". Unfortunately, since
Django's setup.py uses pure distutils (not setuptools), installing
Django with "python setup.py install" does not record any metadata along
with the installation, making an automated uninstall impossible. So the
only workable technique is to manually remove the "django" directory
from site-packages.

I agree that we should add a documentation warning about this. I think
the current warning is not quite in the right place, as it follows the
section on installing via a pth file or symbolic link. "setup.py
install" should not be used in that case, but not for reasons of the
above bug, rather simply because it's not needed. So (IMO) it's
confusing to combine the warnings.

I've filed #18115 to track this.

Carl



signature.asc
Description: OpenPGP digital signature

Re: extra files in startproject

[Alex Ogier]

That seems like too much to ask. "setup.py install" should Just Work(tm),
and if django requires that a certain directory be clean of any .py files
in order to function properly then "setup.py install" should make that
happen. The note might still be valuable, because we should inform people
that untar-ing a version on top of another can cause problems like this,
but breaking "setup.py install" shouldn't be necessary.

If none of these are acceptable, then we can always use a different
directory (django/conf/project_template_new), so that conflicts don't
happen. It's a bit hacky but it would work.

Best,
Alex Ogier

On Thu, Apr 12, 2012 at 5:16 PM, Aymeric Augustin <
aymeric.augus...@polytechnique.org> wrote:

> On 12 avr. 2012, at 21:16, Carl Meyer wrote:
>
> > The open question is just how this situation occurs in the first place.
> > In other words, which particular buggy installer or installation
> > technique is causing an overlaid installation like that, so we can warn
> > people away from it and better advise them how to fix it.
>
> This problem occurs at least when you run "setup.py install" before and
> after the commit that introduced the new project layout.
>
> Some people who had the habit of running "setup.py install" from a git
> clone to keep up-to-date with the development version reported the problem.
> (Just to be 100% clear — this technique doesn't work because it doesn't
> remove .py or .pyc files that are removed from Django.)
> As a result, I added a warning to the docs in r17636.
>
> Most likely, installing 1.4 with "setup.py install", as explained in our
> docs [1], has the same result when 1.3 was previously installed in the same
> fashion.
>
> I suggest we add a warning to this section of the docs: "if a previous
> version of Django is installed, go delete it manually in your site-packages
> directory" (unless there's a better method to remove a Python package?)
>
> Best regards,
>
> --
> Aymeric.
>
> [1]
> https://docs.djangoproject.com/en/dev/topics/install/#installing-an-official-release-manually
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Aymeric Augustin]

On 12 avr. 2012, at 21:16, Carl Meyer wrote:

> The open question is just how this situation occurs in the first place.
> In other words, which particular buggy installer or installation
> technique is causing an overlaid installation like that, so we can warn
> people away from it and better advise them how to fix it.

This problem occurs at least when you run "setup.py install" before and after 
the commit that introduced the new project layout.

Some people who had the habit of running "setup.py install" from a git clone to 
keep up-to-date with the development version reported the problem.
(Just to be 100% clear — this technique doesn't work because it doesn't remove 
.py or .pyc files that are removed from Django.)
As a result, I added a warning to the docs in r17636.

Most likely, installing 1.4 with "setup.py install", as explained in our docs 
[1], has the same result when 1.3 was previously installed in the same fashion.

I suggest we add a warning to this section of the docs: "if a previous version 
of Django is installed, go delete it manually in your site-packages directory" 
(unless there's a better method to remove a Python package?)

Best regards,

-- 
Aymeric.

[1] 
https://docs.djangoproject.com/en/dev/topics/install/#installing-an-official-release-manually

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #18094: signals, model inheritance, and proxy models

[Anssi Kääriäinen]

On Apr 12, 9:58 pm, Carl Meyer  wrote:
> So this is an argument against firing the init signals multiple times,
> for each superclass, but it's not an argument against changing the
> signal framework to include superclass receivers, as proposed in #9318;
> that would not change the performance characteristics for the
> no-receivers situation.
>
> (I see that this choice also has implications for the performance
> improvement patch in #16679, but it looks like we have a workable option
> there either way).

Maybe the solution here is to handle __init__ signals specially - no
inheritance there, and you could then add the signals directly to
model._meta.pre/post_init_signals. Speed would improve, and as far as
I understand these signals are not needed for anything else than
fields doing special setup for related variables (ImageField setting
width and height for example).

Of course, if the receivers caching from #16679 would get in then the
init signals wouldn't be a speed problem. But I would hope that #16679
would not be needed at all if init signals were handled specially, as
other signals do not have any meaningful overhead (warning: haven't
benchmarked). #16679 is making a complex piece of code even more
complex.

> Yes, this is similar to the audit-trail handler I mentioned, and I agree
> that it is a significant backwards-compatibility problem for either option.
>
> So perhaps we do need the signal inheritance behavior to be opt-in when
> connecting the signal handler. I think I'd like to see a deprecation
> path so that eventually the inheritance behavior is the default, and you
> have to opt out of it, as I think in most cases it's the desired behavior.

Unfortunately this seems to be the only backwards compatible way
forward. I don't know how to do the technical details... from
__future__ import pre_save? :)

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #18094: signals, model inheritance, and proxy models

[Alex Ogier]

I think changing when signals fire is bound to cause breakages for some
apps, and of the worst variety because signals both deal with basic data
integrity and are relatively opaque (I.e. debugging is a pain). Even if the
current behavior isn't what we would choose given a blank slate, it can't
realistically be changed.

On the other hand, supporting thus use case isn't impossible. Maybe we
could add a "fire_for_subclasses" kwarg to the connect() method or
something. That way concrete models can specify that they absolutely want
their signals fired, and the resolution can be done at load time to avoid
performance hits from isinstance calls.

Best,
Alex Ogier
On Apr 12, 2012 2:59 PM, "Carl Meyer"  wrote:

> On 04/12/2012 12:43 PM, Anssi Kääriäinen wrote:
> > It is important that pre/post init signals will not get more expensive
> > than they currently are. Even now they can give around 100% overhead
> > to model.__init__(). And this is in a case where the currently
> > initialized model has no signals attached at all - it is enough that
> > _some_ model in the project has pre or post init signals attached. I
> > don't believe signaling cost is severe for .save and .delete as those
> > operations are doing much heavier work than __init__ anyways, but for
> > init the cost is big enough already. If at all possible, make them
> > cheaper.
>
> So this is an argument against firing the init signals multiple times,
> for each superclass, but it's not an argument against changing the
> signal framework to include superclass receivers, as proposed in #9318;
> that would not change the performance characteristics for the
> no-receivers situation.
>
> (I see that this choice also has implications for the performance
> improvement patch in #16679, but it looks like we have a workable option
> there either way).
>
> > In addition, just changing the signals to fire for every parent class
> > is pretty much as backwards incompatible as it gets: imagine you have
> > post_save signal which increments a counter in a related model on
> > create (posts in a thread for example). You have also a proxy model,
> > and you have attached the same signal to the proxy model, too, because
> > that is what you have to do to get the signal fire when you save the
> > proxy model. After the change you will get the counter incremented two
> > times for create, and you have a data corrupting bug. This might not
> > be a common pattern, but for example Django's use of pre and post init
> > signals follows the pattern of attaching to each subclass individually
> > and these signals would thus fire multiple times.
>
> Yes, this is similar to the audit-trail handler I mentioned, and I agree
> that it is a significant backwards-compatibility problem for either option.
>
> So perhaps we do need the signal inheritance behavior to be opt-in when
> connecting the signal handler. I think I'd like to see a deprecation
> path so that eventually the inheritance behavior is the default, and you
> have to opt out of it, as I think in most cases it's the desired behavior.
>
> Carl
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject

[Carl Meyer]

Hi Alex,

On 04/12/2012 01:07 PM, Alex Ogier wrote:
> Maybe it would be worth experimenting with various combinations of
> django 1.x django-admin.py executables with django 1.4 libraries? Maybe
> if django-admin.py is a symlink into a 1.3 tree but django 1.4 is on the
> search path this stuff could crop up?

Sorry, I wasn't clear - I already know what the immediate cause of the
problem is, and it's easy to reproduce manually.

"startproject" takes the actual files in django/conf/project_template/
as the template for the new project. If an installation of Django 1.4 is
installed over top of an installation of Django 1.3 (and I mean
literally on top of, in the same filesystem tree), then the project
template files that were moved to a subdirectory in 1.4 (__init__.py,
urls.py, settings.py) are found in both locations. So even if it's
purely the 1.4 startproject code running, it'll install these extra
files, because it finds them there in the project template.

The open question is just how this situation occurs in the first place.
In other words, which particular buggy installer or installation
technique is causing an overlaid installation like that, so we can warn
people away from it and better advise them how to fix it.

Carl



signature.asc
Description: OpenPGP digital signature

Re: extra files in startproject (was: Django is not a serious framework, really)

[Alex Ogier]

Maybe it would be worth experimenting with various combinations of django
1.x django-admin.py executables with django 1.4 libraries? Maybe if
django-admin.py is a symlink into a 1.3 tree but django 1.4 is on the
search path this stuff could crop up?

Best,
Alex Ogier
On Apr 12, 2012 2:32 PM, "Carl Meyer"  wrote:

> Hi Jason,
>
> On 04/11/2012 06:10 AM, Jason Ma wrote:
> > Hi,
> > I download and tried to use the Django 1.4 yesterday. I am a dummy
> > and I just follow the official document, but When I just start a
> > project.
> > I found that it is what I see from my computer:
> >
> > jason@jason-pc:~/workspace/hunqing$ tree .
> > .
> > ├── hunqing
> > │   ├── __init__.py
> > │   ├── __init__.pyc
> > │   ├── settings.py
> > │   ├── settings.pyc
> > │   ├── urls.py
> > │   ├── urls.pyc
> > │   ├── wsgi.py
> > │   └── wsgi.pyc
> > ├── __init__.py
> > ├── manage.py
> > ├── settings.py
> > └── urls.py
> >
> > but what doc say?
> > mysite/
> > manage.py
> > mysite/
> > __init__.py
> > settings.py
> > urls.py
> > wsgi.py
>
> Others have commented on the pyc files, but I don't think that's really
> the point here. The point is that there is an extra __init__.py,
> settings.py, and urls.py in the outer directory next to manage.py that
> should not be there.
>
> This is definitely a bug, and it's one that I've already seen reported
> several times, but it is not a bug in Django. With a clean installation
> of Django 1.4, the tutorial steps work as advertised. This bug occurs if
> somehow your installation of Django 1.4 is "layered" on top of an
> installation of Django 1.3, without the 1.3 installation ever having
> been removed. I'm not sure how this happens, but my best guess is that
> it could happen if you are using a very old version of pip (like, pip
> 0.3 era, before pip gained uninstall support). You're most likely to be
> using an old pip if you are using a Linux distribution's packaged pip;
> those tend to be quite outdated.
>
> If you are interested in helping to solve this issue (as opposed to, for
> instance, trollish hyperbole), it would be very helpful to know what
> operating system and version you are using, and how you installed Django
> 1.4 (i.e the exact commands you used).
>
> Thanks!
>
> Carl
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #18094: signals, model inheritance, and proxy models

[Carl Meyer]

On 04/12/2012 12:43 PM, Anssi Kääriäinen wrote:
> It is important that pre/post init signals will not get more expensive
> than they currently are. Even now they can give around 100% overhead
> to model.__init__(). And this is in a case where the currently
> initialized model has no signals attached at all - it is enough that
> _some_ model in the project has pre or post init signals attached. I
> don't believe signaling cost is severe for .save and .delete as those
> operations are doing much heavier work than __init__ anyways, but for
> init the cost is big enough already. If at all possible, make them
> cheaper.

So this is an argument against firing the init signals multiple times,
for each superclass, but it's not an argument against changing the
signal framework to include superclass receivers, as proposed in #9318;
that would not change the performance characteristics for the
no-receivers situation.

(I see that this choice also has implications for the performance
improvement patch in #16679, but it looks like we have a workable option
there either way).

> In addition, just changing the signals to fire for every parent class
> is pretty much as backwards incompatible as it gets: imagine you have
> post_save signal which increments a counter in a related model on
> create (posts in a thread for example). You have also a proxy model,
> and you have attached the same signal to the proxy model, too, because
> that is what you have to do to get the signal fire when you save the
> proxy model. After the change you will get the counter incremented two
> times for create, and you have a data corrupting bug. This might not
> be a common pattern, but for example Django's use of pre and post init
> signals follows the pattern of attaching to each subclass individually
> and these signals would thus fire multiple times.

Yes, this is similar to the audit-trail handler I mentioned, and I agree
that it is a significant backwards-compatibility problem for either option.

So perhaps we do need the signal inheritance behavior to be opt-in when
connecting the signal handler. I think I'd like to see a deprecation
path so that eventually the inheritance behavior is the default, and you
have to opt out of it, as I think in most cases it's the desired behavior.

Carl



signature.asc
Description: OpenPGP digital signature

Re: #18094: signals, model inheritance, and proxy models

[Anssi Kääriäinen]

It is important that pre/post init signals will not get more expensive
than they currently are. Even now they can give around 100% overhead
to model.__init__(). And this is in a case where the currently
initialized model has no signals attached at all - it is enough that
_some_ model in the project has pre or post init signals attached. I
don't believe signaling cost is severe for .save and .delete as those
operations are doing much heavier work than __init__ anyways, but for
init the cost is big enough already. If at all possible, make them
cheaper.

In addition, just changing the signals to fire for every parent class
is pretty much as backwards incompatible as it gets: imagine you have
post_save signal which increments a counter in a related model on
create (posts in a thread for example). You have also a proxy model,
and you have attached the same signal to the proxy model, too, because
that is what you have to do to get the signal fire when you save the
proxy model. After the change you will get the counter incremented two
times for create, and you have a data corrupting bug. This might not
be a common pattern, but for example Django's use of pre and post init
signals follows the pattern of attaching to each subclass individually
and these signals would thus fire multiple times.

 - Anssi

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: extra files in startproject (was: Django is not a serious framework, really)

[Carl Meyer]

Hi Jason,

On 04/11/2012 06:10 AM, Jason Ma wrote:
> Hi,
> I download and tried to use the Django 1.4 yesterday. I am a dummy
> and I just follow the official document, but When I just start a
> project.
> I found that it is what I see from my computer:
> 
> jason@jason-pc:~/workspace/hunqing$ tree .
> .
> ├── hunqing
> │   ├── __init__.py
> │   ├── __init__.pyc
> │   ├── settings.py
> │   ├── settings.pyc
> │   ├── urls.py
> │   ├── urls.pyc
> │   ├── wsgi.py
> │   └── wsgi.pyc
> ├── __init__.py
> ├── manage.py
> ├── settings.py
> └── urls.py
> 
> but what doc say?
> mysite/
> manage.py
> mysite/
> __init__.py
> settings.py
> urls.py
> wsgi.py

Others have commented on the pyc files, but I don't think that's really
the point here. The point is that there is an extra __init__.py,
settings.py, and urls.py in the outer directory next to manage.py that
should not be there.

This is definitely a bug, and it's one that I've already seen reported
several times, but it is not a bug in Django. With a clean installation
of Django 1.4, the tutorial steps work as advertised. This bug occurs if
somehow your installation of Django 1.4 is "layered" on top of an
installation of Django 1.3, without the 1.3 installation ever having
been removed. I'm not sure how this happens, but my best guess is that
it could happen if you are using a very old version of pip (like, pip
0.3 era, before pip gained uninstall support). You're most likely to be
using an old pip if you are using a Linux distribution's packaged pip;
those tend to be quite outdated.

If you are interested in helping to solve this issue (as opposed to, for
instance, trollish hyperbole), it would be very helpful to know what
operating system and version you are using, and how you installed Django
1.4 (i.e the exact commands you used).

Thanks!

Carl



signature.asc
Description: OpenPGP digital signature

Re: #18094: signals, model inheritance, and proxy models

[Javier Guerra Giraldez]

On Thu, Apr 12, 2012 at 12:19 PM, Carl Meyer  wrote:
> Also, it isn't really true that the model signals are strictly tied to
> database activity; they are tied to events on Python model objects. One
> of the three signals under discussion is the pre/post_init signal, which
> fires anytime a model instance is instantiated, even if no database
> query or database change occurred.

i think this is the semantic issue that should be explicit:

if those signals are for _every_ model, then should fire for abstract
parents and abstract children (proxy models).

maybe a 'fire_signals' meta option that defaults to False on abstract
parents and True on proxy (and normal) models?

-- 
Javier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #18094: signals, model inheritance, and proxy models

[Carl Meyer]

On 04/12/2012 11:02 AM, Javier Guerra Giraldez wrote:
> IOW, i think the existing signals are database-related and should be
> fired only for the concrete part(s).  if the abstract part wants to,
> it can send custom signals.

Also, it isn't really true that the model signals are strictly tied to
database activity; they are tied to events on Python model objects. One
of the three signals under discussion is the pre/post_init signal, which
fires anytime a model instance is instantiated, even if no database
query or database change occurred.

Carl



signature.asc
Description: OpenPGP digital signature

Re: #18094: signals, model inheritance, and proxy models

[Carl Meyer]

On 04/12/2012 11:02 AM, Javier Guerra Giraldez wrote:
> in my mental model, there are three types of inheritance:
> 
> concrete: two tables, deletion should fire two signals, one for the
> child record and one for the parent record.
> 
> abstract: there's no parent table, deletion should fire one signal,
> for the child record; unless the parent overrides the delete() method
> and chooses to send a custom signal
> 
> proxy: there's no child table.  kind of an 'abstract child', instead
> of 'abstract parent'.  deletion should fire one signal, for the
> _parent_ record (which is the only real record), unless the child
> overrides the delete() method and chooses to send a custom signal.
> 
> 
> IOW, i think the existing signals are database-related and should be
> fired only for the concrete part(s).  if the abstract part wants to,
> it can send custom signals.

I agree that signals should not be fired for abstract models. Proxy
models are a rather different case, though - they exist precisely in
order to be able to augment/supplement Python-level model behavior, and
signals are a part of that behavior. It would seem unfortunate to me if
you couldn't attach signal handlers specifically to a proxy subclass of
a model.

Also, firing signals only for the concrete superclass of a proxy model
would be a much worse backwards-incompatible change than any of the
others discussed. Signal handlers (for delete, save, or init) registered
for a proxy model class specifically currently work - with this change
they would just suddenly stop firing.

> second idea:
> 
> define three different signals: one for concrete members, one for
> abstract, and one for both.
> 
> - non-inheritance deletion:
> - send concrete-deleted and common-deleted
> 
> - concrete inheritance deletion (both are concrete):
> - child: concrete-deleted and common-deleted
> - parent: concrete-deleted and common-deleted
> 
> - abstract inheritance (parent is abstract):
> - child: concrete-deleted and common-deleted
> - parent: abstract-deleted and common-deleted
> 
> - proxy inheritance (child is abstract):
> - child: abstract-deleted and common-deleted
> - parent: concrete-deleted and common-deleted
> 
> but i think that's overkill

Yes, I think this is too much complexity compared to the other solutions.

Carl



signature.asc
Description: OpenPGP digital signature

Re: #18094: signals, model inheritance, and proxy models

[Carl Meyer]

On 04/12/2012 10:52 AM, Jeremy Dunck wrote:
> On Thu, Apr 12, 2012 at 9:31 AM, Carl Meyer  wrote:
>> There's a discussion ongoing on ticket #18094
>> (https://code.djangoproject.com/ticket/18094) that has enough potential
>> back-compat implications that it seems worth getting feedback here.
> 
> Small note, I'll try to respond to the whole thing later.  This ticket
> exists on the topic, too:
> https://code.djangoproject.com/ticket/9318

Thanks for pointing that out! I've closed #18094 as a duplicate of
#9318, as they really are addressing the same issue. #9318 is currently
taking the approach I'd dismissed as "not a realistic option" of fixing
the signals framework itself to be smart about sender subclasses. It may
be that I dismissed that option too quickly.

Carl



signature.asc
Description: OpenPGP digital signature

Re: #18094: signals, model inheritance, and proxy models

[Javier Guerra Giraldez]

On Thu, Apr 12, 2012 at 11:31 AM, Carl Meyer  wrote:
> Thoughts?

in my mental model, there are three types of inheritance:

concrete: two tables, deletion should fire two signals, one for the
child record and one for the parent record.

abstract: there's no parent table, deletion should fire one signal,
for the child record; unless the parent overrides the delete() method
and chooses to send a custom signal

proxy: there's no child table.  kind of an 'abstract child', instead
of 'abstract parent'.  deletion should fire one signal, for the
_parent_ record (which is the only real record), unless the child
overrides the delete() method and chooses to send a custom signal.


IOW, i think the existing signals are database-related and should be
fired only for the concrete part(s).  if the abstract part wants to,
it can send custom signals.


second idea:

define three different signals: one for concrete members, one for
abstract, and one for both.

- non-inheritance deletion:
- send concrete-deleted and common-deleted

- concrete inheritance deletion (both are concrete):
- child: concrete-deleted and common-deleted
- parent: concrete-deleted and common-deleted

- abstract inheritance (parent is abstract):
- child: concrete-deleted and common-deleted
- parent: abstract-deleted and common-deleted

- proxy inheritance (child is abstract):
- child: abstract-deleted and common-deleted
- parent: concrete-deleted and common-deleted

but i think that's overkill

-- 
Javier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #18094: signals, model inheritance, and proxy models

[Jeremy Dunck]

On Thu, Apr 12, 2012 at 9:31 AM, Carl Meyer  wrote:
> Hi all,
>
> There's a discussion ongoing on ticket #18094
> (https://code.djangoproject.com/ticket/18094) that has enough potential
> back-compat implications that it seems worth getting feedback here.

Small note, I'll try to respond to the whole thing later.  This ticket
exists on the topic, too:
https://code.djangoproject.com/ticket/9318

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

#18094: signals, model inheritance, and proxy models

[Carl Meyer]

Hi all,

There's a discussion ongoing on ticket #18094
(https://code.djangoproject.com/ticket/18094) that has enough potential
back-compat implications that it seems worth getting feedback here.

Currently, when you delete a concrete-inheritance child model instance,
pre_delete and post_delete signals are sent both for the child and
parent models.

In contrast, when you delete a proxy model instance, the pre/post_delete
signals are sent only for the proxy model class, never for the parent
concrete class.

This is problematic. Imagine a reusable tagging app that has a Tag
model, and attaches a pre_delete signal handler for Tag instances. In my
usage of that app, I use my own TagProxy subclass. When I delete a
TagProxy instance, a Tag is in fact deleted, but the app's pre_delete
signal handler never fires. This violates the reasonable assumption that
subclassing doesn't change superclass behavior except where it
explicitly overrides it.

So we'd like to make the obvious fix, which is to fire the
pre/post_delete signals for every superclass of the instance(s) you
delete, regardless of whether they are parents via concrete or proxy
inheritance.

This raises the question of consistency with pre/post_save and
pre/post_init, which are currently both sent only once, for the exact
class being saved/initialized (thus already inconsistent with delete
signals, which are also sent for concrete-inheritance parents). The same
Tag scenario above would apply equally to save and init signals: the
tagging app should be able to assume that if it registers a save or init
signal handler for Tag, it will get called whenever a Tag is saved or
initialized, even if that's happening via a Tag subclass.

So it seems that perhaps we should also fix the save and init signals to
be fired for each superclass. Is this an acceptable change from a
backwards-compatibility perspective? In the common case of registering a
signal handler using "sender=", it should be a clear win: signal
handlers will now execute when you'd expect them to. But in the case of,
say, a generic audit-trail handler that listens for all post_save (no
sender specified), it will now get what could seem like duplicate
signals for inherited models (one for each superclass of the instance
being saved).

Thoughts?

(The other approach that might come to mind for fixing this would be to
move the fix into the signals framework itself, so that it executes
signal handlers registered for signal=Foo anytime a signal is fired for
anything that passes issubclass(Foo), rather than requiring Foo
identity. This is appealing, and would solve the potential "duplicate
signals" problem with a generic receiver, but is a much more invasive
change in the fundamental semantics of the signals framework, so I don't
think it's a realistic option.)

Carl



signature.asc
Description: OpenPGP digital signature