Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
--+
 Reporter:  Simon Charette|Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  closed
Component:  Migrations|  Version:  master
 Severity:  Normal|   Resolution:  wontfix
 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 Claude Paroz):

 Thanks for sharing your experimentations!
 "Le mieux est parfois l'ennemi du bien" :-)

-- 
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/067.b19569a08f36c5060edd7293825bda03%40djangoproject.com.


Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
--+
 Reporter:  Simon Charette|Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  closed
Component:  Migrations|  Version:  master
 Severity:  Normal|   Resolution:  wontfix
 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 Simon Charette):

 * status:  new => closed
 * resolution:   => wontfix


Comment:

 After a bit of investigation it looks like this would effectively add a
 lot of complexity to the already complex `_alter_field` for little benefit
 because the optimization is only valuable when only performing a
 `(db_index=True)` to `(unique=True)` migration. If the field type is also
 meant to be altered (e.g type) we certainly want to drop the index before
 hand to avoid an internal rebuild with the new type which makes it hard to
 determine when the optimization should be performed.

 For the record I managed to get a noticeable speed up (10-15%) on
 PostgreSQL when adding a unique constraint to an already indexed column
 (10M rows integer).

-- 
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/067.fe6dc8fbce6d8ae69b6d69b32d3f4635%40djangoproject.com.


[Django] #30870: "migrate --plan" outputs "IRREVERSIBLE" on RunPython operation when no docstring present

2019-10-10 Thread Django
#30870: "migrate --plan" outputs "IRREVERSIBLE" on RunPython operation when no
docstring present
-+-
   Reporter: |  Owner:  nobody
  kdickerson |
   Type:  Bug| Status:  new
  Component:  Core   |Version:  2.2
  (Management commands)  |
   Severity:  Normal |   Keywords:  migrate
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  1
  UI/UX:  0  |
-+-
 Given a migration like:
 {{{#!python
 from django.db import migrations

 def forward(apps, schema_editor):
 pass

 def reverse(apps, schema_editor):
 pass

 class Migration(migrations.Migration):
 operations = [
 migrations.RunPython(forward, reverse)
 ]
 }}}

 `manage.py migrate --plan` will output:

 {{{
 Planned operations:
 example.0001_initial
 Raw Python operation -> IRREVERSIBLE
 }}}

 The migration should not be described as "irreversible".

 This error is in the definition of `describe_operation` in
 `django/django/core/management/commands/migrate.py`, reproduced below with
 line numbers from 2.2.6 tag.

 {{{#!python
 343@staticmethod
 344def describe_operation(operation, backwards):
 345"""Return a string that describes a migration operation for
 --plan."""
 346prefix = ''
 347if hasattr(operation, 'code'):
 348code = operation.reverse_code if backwards else
 operation.code
 349action = code.__doc__ if code else ''
 350elif hasattr(operation, 'sql'):
 351action = operation.reverse_sql if backwards else
 operation.sql
 352else:
 353action = ''
 354if backwards:
 355prefix = 'Undo '
 356if action is None:
 357action = 'IRREVERSIBLE'
 358is_error = True
 359else:
 360action = str(action).replace('\n', '')
 361is_error = False
 362if action:
 363action = ' -> ' + action
 364truncated = Truncator(action)
 365return prefix + operation.describe() + truncated.chars(40),
 is_error
 }}}


 Line 349 uses the docstring as the output string.
 Line 356 tests that value and sets `action = 'IRREVERSIBLE'` on line 357
 because the dosctring is `None`.

 It would appear that the intention is to use a docstring to describe the
 operation, if available, and leave it blank otherwise.  However, because
 it tests against `code` instead of `code.__doc__` it actually sets `action
 = None` resulting in 'IRREVERSIBLE' being displayed.

 == Proposed Solutions below

 For a localized fix, I believe line 349 should be replaced by
 {{{#!python
 if code:
 action = code.__doc__ if code.__doc__ else ''
 else:
 action = None
 }}}

 However, a more holistic view suggests that displaying "IRREVERSIBLE"
 isn't really the correct thing to do.  "IRREVERSIBLE" is set when
 `is_error` is also set to `True` and seems to be trying to indicate that
 the migration operation is invalid rather than irreversible.  That is, if
 `code`/`reverse_code` is None (line 348) or `sql`/`reverse_sql` is None
 (line 351) the migration can't run.

 Since `sql` and `code` are required parameters for their respective
 Operations, `action` should only possibly be None in the reverse case,
 which seems to be what this code is trying to capture and explain.

 Given that, a better approach would probably make use of the `reversible`
 property defined on `RunSQL` and `RunPython` operations.  This is a little
 verbose and could probably be pared down, but I think it has the right
 logic:

 {{{#!python
 @staticmethod
 def describe_operation(operation, backwards):
 """Return a string that describes a migration operation for --plan."""
 prefix = ''
 action = ''
 is_error = False

 if backwards:
 prefix = 'Undo '
 if hasattr(operation, 'reversible') and not operation.reversible:
 action = 'INVALID'
 is_error = True
 elif hasattr(operation, 'reverse_code'):
 action = operation.reverse_code.__doc__ if
 operation.reverse_code.__doc__ else ''
 elif hasattr(operation, 'reverse_sql'):
 action = operation.reverse_sql.__doc__ if
 operation.reverse_sql.__doc__ else ''
 else:
 if hasattr(operation, 'code'):
 action = operation.code.__doc__ if operation.code.__doc__ else
 ''
 elif hasattr(operation, 'sql'):
 action = operation.sql.__doc__ if operation.sql.__doc__ else
 ''

 action = ' -> ' + str(action).replace('\n', '')
 truncated = Tru

[Django] #30869: Setting to confirm destructive/weird migrations.

2019-10-10 Thread Django
#30869: Setting to confirm destructive/weird migrations.
-+-
   Reporter:  awiebe |  Owner:  nobody
   Type:  New| Status:  new
  feature|
  Component: |Version:  2.2
  Migrations |
   Severity:  Normal |   Keywords:  destructive,setting
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+-
 If altering or deleting a column could result in loss of data the migrate
 command should inform you, unless you have set a flag in the migration
 file that says you have provided an appropriate data migration.

 When iterating, on non production code it's obviously fine if data is
 added and removed because you're just using test data anyway, but
 migrations make their way into production code, and then suddenly the fact
 that you deleted a column as part of an intermediate migration matters,
 even if you provided a replacement at some later point.  I would put a
 setting that prevents django from executing:

 * Delete Model
 * Remove Field
 * Alter Field

 Without a review of the migration. This could be a setting but maybe it
 should always be on.

 If possible it would also be helpful if django would automatically split
 destructive operations and non destructive operations into two steps.  For
 example.

 If I add a column that will contain the migrated data, and remove the
 column that held the old data, the operation to create and populate the
 new column needs to happen first.  I don't know if the migrator already
 handles this but iff not, then destructive operation should be pushed into
 a separate subsequent migration, where the "data migration provided" flag
 is already set.

 I had this problem when I was trying to change a string field to a foreign
 key, and the migrator just turned my sqlite column from a string column to
 an integer, which is perfectly legal in sqlite for some reason.  But
 didn't actually I should have made a data migration that I didn't notice.

 This was on a test database, but I essentially had to unpick weird things
 sqlite did, which could have been avoided if django had been a bit more
 conservative about migrating.

-- 
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/049.a5af0ff520c6992b591a611b8a247c1e%40djangoproject.com.


Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
--+
 Reporter:  Simon Charette|Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Migrations|  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 Simon Charette):

 > I would also add that at some point, you should assume the fact you are
 using a database not supporting transactional DDL.

 FWIW I'm trying to figure out if it does make a difference on backends
 that support transactional DDL as well.

 For example, it's not completely crazy to assume that PostgreSQL could use
 an existing index on a column it's trying to create a unique constraint on
 to speed up the operation given B-trees are used for both implementations.
 If that's the case deferring the index removal could be beneficial to
 other backends as well.

-- 
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/067.69f12e87867314d001c0615f574c2f7c%40djangoproject.com.


Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
--+
 Reporter:  Simon Charette|Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Migrations|  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 Claude Paroz):

 I would also add that at some point, you should assume the fact you are
 using a database not supporting transactional DDL.

-- 
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/067.44cc7a05fe9ae18ef14e35e4dc304cee%40djangoproject.com.


Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
-+-
 Reporter:  Simon Charette   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Claude Paroz):

 > Is there any reason except for double index storage needs not to do it?

 No, if we can reasonably estimate that the risk of introducing regressions
 is low and that it doesn't add too much code complexity.

-- 
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/067.ec7b8178bba4549ce413abac1a937ccc%40djangoproject.com.


Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
--+
 Reporter:  Simon Charette|Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Migrations|  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 Claude Paroz):

 * stage:  Unreviewed => Accepted


-- 
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/067.6b50b654e231a002d40f94ff4468e0e3%40djangoproject.com.


Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
-+-
 Reporter:  Simon Charette   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 > Why do you see that as a problem? Shouldn't the time between the two
 commands be rather short?

 Unique constraint creation can be relatively slow on large tables
 particularly when performed without a lock (e.g. MySQL `LOCK=NONE`) to
 allow concurrent read and writes. During that time period not having an
 index on a frequently queried column can cause such queries to take a a
 few orders more time to process and thus slowdown the constraint creation
 time even more. At least that's what we've experienced on a table with 6M
 rows when adding a unique constraint on a `VARCHAR(255) NOT NULL`.

 > I think that reduced performance is something we could expect during the
 execution of a migration.

 Agreed but if we can reduce the performance degradation at the cost of
 temporarily elevated storage needs I think we should do it.

 Is there any reason except for double index storage needs not to do it?

-- 
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/067.d05e311bb3fdc01989bdb67029ec43c6%40djangoproject.com.


Re: [Django] #30854: Selecting multiple FilteredRelation's returns only the last one.

2019-10-10 Thread Django
#30854: Selecting multiple FilteredRelation's returns only the last one.
-+-
 Reporter:  Morteza PRK  |Owner:  Hasan
 |  Ramezani
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  FilteredRelation | Triage Stage:  Accepted
  annotate   |
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Hasan Ramezani):

 * has_patch:  0 => 1


-- 
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/068.b9e89dc09a403c11f380b13ba320f846%40djangoproject.com.


Re: [Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
-+-
 Reporter:  Simon Charette   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Claude Paroz):

 Why do you see that as a problem? Shouldn't the time between the two
 commands be rather short? I think that reduced performance is something we
 could expect during the execution of a migration.

-- 
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/067.a7b4367aa1440cca93f8221564a3ba34%40djangoproject.com.


Re: [Django] #30779: technical_500.html: use filename:lineno format for exception location

2019-10-10 Thread Django
#30779: technical_500.html: use filename:lineno format for exception location
--+
 Reporter:  Daniel Hahler |Owner:  (none)
 Type:  Cleanup/optimization  |   Status:  new
Component:  Error reporting   |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  1
Easy pickings:  0 |UI/UX:  0
--+
Changes (by vinayak-sharda):

 * owner:  vinayak-sharda => (none)
 * status:  assigned => new


-- 
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/065.2adc664bb58721749b624a4da8ef635e%40djangoproject.com.


Re: [Django] #30779: technical_500.html: use filename:lineno format for exception location

2019-10-10 Thread Django
#30779: technical_500.html: use filename:lineno format for exception location
-+-
 Reporter:  Daniel Hahler|Owner:  vinayak-
 Type:   |  sharda
  Cleanup/optimization   |   Status:  assigned
Component:  Error reporting  |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by vinayak-sharda):

 * owner:  (none) => vinayak-sharda
 * 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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.5f81e66fae44643506ed7b9d612dc5e1%40djangoproject.com.


[Django] #30868: ForeignKey's "to_field" parameter gets the old field's name when renaming a PrimaryKey

2019-10-10 Thread Django
#30868: ForeignKey's "to_field" parameter gets the old field's name when 
renaming a
PrimaryKey
-+-
   Reporter:  Carlos E.  |  Owner:  nobody
  C. Leite   |
   Type:  Bug| Status:  new
  Component: |Version:  2.2
  Migrations |
   Severity:  Release|   Keywords:  migration
  blocker|  renamefield to_field
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  1
  UI/UX:  0  |
-+-
 Having these two models

 {{{
 class ModelA(models.Model):
 field_wrong = models.CharField('field1', max_length=50,
 primary_key=True)  # I'm a Primary key.

 }}}

 {{{
 class ModelB(models.Model):
 field_fk = models.ForeignKey(ModelA, blank=True, null=True,
 on_delete=models.CASCADE)
 }}}

 ... migrations applyed ...

 the `ModelA.field_wrong` field has been renamed ... and Django recognizes
 the "renaming"

 {{{
 # Primary key renamed
 class ModelA(models.Model):
 field_fixed = models.CharField('field1', max_length=50,
 primary_key=True)  # I'm a Primary key.

 }}}

 Attempts to [[span(style=color: #FF, to_field)]] parameter.

 The **to_field** points to the **old_name** (**field_typo**) and not to
 the new one ("**field_fixed**")

 {{{
 class Migration(migrations.Migration):

 dependencies = [
 ('app1', '0001_initial'),
 ]

 operations = [
 migrations.RenameField(
 model_name='modela',
 old_name='field_wrong',
 new_name='field_fixed',
 ),
 migrations.AlterField(
 model_name='modelb',
 name='modela',
 field=models.ForeignKey(blank=True, null=True,
 on_delete=django.db.models.deletion.CASCADE, to='app1.ModelB',
 to_field='field_wrong'),
 ),
 ]

 }}}

-- 
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/053.7d1f1f154cfef089524ab9b0a7125fc2%40djangoproject.com.


Re: [Django] #30563: Optimize django.forms.widgets.Media.__add__.

2019-10-10 Thread Django
#30563: Optimize django.forms.widgets.Media.__add__.
--+
 Reporter:  David Dorothy |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Forms |  Version:  master
 Severity:  Normal|   Resolution:
 Keywords:  media | Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by David Dorothy):

 I apologize for my delay in responding to this. I had to switch tasks for
 a little while to a different project that took a bit longer than
 intended.

 I have done some further research into this issue and have some
 information based on the actual use. I have 194016 Media objects that are
 being added together.  For the JavaScript references when the lists are
 de-duplicated in the way you describe above (in `__add__` not in `merge`)
 the list is trimmed to 13 records. CSS is handled differently and I'm not
 sure it is correct (see code later in this comment). This provides a
 performance on my machine of around 3-5 seconds. This is not as fast as my
 original optimization but is definitely close enough.

 As far as sharing code, I cannot give you access to my repository.
 However, I can show you the code where the adding of Media objects is
 occurring at.

 First, here is my attempt at improving the `Media.__add__` method:

 {{{#!python
 def combine_css(destination, css_list):
 for x in filter(None, css_list):
 for key in x.keys():
 if key not in destination:
 destination[key] = OrderedSet()
 for item in x[key]:
 if item:
 destination[key].add(item)


 class ImprovedMedia(forms.Media):

 def __add__(self, other):
 combined = ImprovedMedia()
 combined._css_lists = list(filter(None, self._css_lists +
 other._css_lists))
 css_lists = {}
 combine_css(css_lists, self._css_lists)
 combine_css(css_lists, other._css_lists)
 combined._css_lists = [css_lists]

 combined._js_lists = list(OrderedSet([tuple(x) for x in
 filter(None, self._js_lists + other._js_lists)]))
 return combined
 }}}

 I am concerned that my `combine_css()` function may not be the best
 implementation because it does keep order but it always results in exactly
 one new `self._css_lists` which may not be the correct solution. I don't
 think I have enough context to know for sure if this is right. Also, I
 have some reservations about how I use `OrderedSet` for `Media._css_lists`
 because in my initial attempts on the `Media._js_lists` I had to convert
 the `OrderedSet` back into a `list` to make things work.

 Second, here is the code where the `__add__` method is called:

 {{{#!python
 class StructuredStreamBlock(StreamBlock):
 js_classes = {}

 # ...

 def all_media(self):
 media = ImprovedMedia()
 all_blocks = self.all_blocks()
 for block in all_blocks:
 media += block.
 return media

 # ...
 }}}

 The original code can be found in wagtail's source code here:
 
https://github.com/wagtail/wagtail/blob/e1d3390a1f62ae4b30c0ef38e7cfa5070d8565dc/wagtail/core/blocks/base.py#L74

 Unfortunately I could not think of a way to monkey patch the Media class
 in Django without actually changing the Django code in my virtual
 environment.

 Should I attempt at this point to create a fork and a branch (and
 hopefully eventually a pull request) and see if the change I make break
 any existing tests or should some others with more familiarity weigh in on
 this first?

-- 
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/067.f0e95627feb7779ca573c5730aad8121%40djangoproject.com.


Re: [Django] #23755: patch_cache_control should special case "no-cache"

2019-10-10 Thread Django
#23755: patch_cache_control should special case "no-cache"
-+-
 Reporter:  thenewguy|Owner:  Flavio
 |  Curella
 Type:  New feature  |   Status:  closed
Component:  Core (Cache system)  |  Version:  master
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"ed112fadc1cfa400dbee6080bf82fd7536ea4c72" ed112fad]:
 {{{
 #!CommitTicketReference repository=""
 revision="ed112fadc1cfa400dbee6080bf82fd7536ea4c72"
 Fixed #23755 -- Added support for multiple field names in the no-cache
 Cache-Control directive to patch_cache_control().

 https://tools.ietf.org/html/rfc7234#section-5.2.2.2
 }}}

-- 
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/067.060bfc3797bf8b855fb0f9360f928731%40djangoproject.com.


Re: [Django] #20393: django.db.models.query.QuerySet.__repr__ should not have side-effects

2019-10-10 Thread Django
#20393: django.db.models.query.QuerySet.__repr__ should not have side-effects
-+-
 Reporter:  justin@… |Owner:  nobody
 Type:  Uncategorized|   Status:  closed
Component:  Database layer   |  Version:  1.4
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:  QuerySet, repr,  | Triage Stage:
  side-effect|  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Matt Johnson):

 I opened a discussion on the dev mailing list about this:

 https://groups.google.com/forum/#!topic/django-developers/WzRZ0IfBipc

-- 
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/079.b991fc573bc62d82156584617c89ad2f%40djangoproject.com.


[Django] #30867: Old indexes should be dropped after new ones are created.

2019-10-10 Thread Django
#30867: Old indexes should be dropped after new ones are created.
+
   Reporter:  Simon Charette|  Owner:  nobody
   Type:  Cleanup/optimization  | Status:  new
  Component:  Migrations|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 |
+
 When adding `unique=True` to a field previously `index=True` field or
 vice-versa the old index is dropped before the new one is added.

 This is not a big issue on backends that support transactional DDL but it
 is on ones that don't because in between the execution of the two
 statements the column is not indexed.

 For example given the following model

 {{{#!python
 class Author(models.Model):
 name = models.CharField(max_length=50, db_index=True)
 }}}

 Altering `name` to `models.CharField(max_length=50, unique=True)` will
 generate the following SQL on MySQL

 {{{#!sql
 DROP INDEX `author_name_idx` ON `author`;
 -- Until the following statement completes the name field is not indexed
 at all.
 -- This can take a while on large tables.
 ALTER TABLE `author` ADD CONSTRAINT `author_name_uniq` UNIQUE (`name`);
 }}}

-- 
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/052.099fe91c28b50070f0fef6928c0fe9ec%40djangoproject.com.


Re: [Django] #30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py

2019-10-10 Thread Django
#30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py
--+
 Reporter:  Carlton Gibson|Owner:  (none)
 Type:  Cleanup/optimization  |   Status:  assigned
Component:  Error reporting   |  Version:  2.2
 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 Carlton Gibson):

 OK, thanks Claude!

-- 
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/071.df92aa58737c8486ac14a8c9cf917fab%40djangoproject.com.


Re: [Django] #30859: Enable the supports_aggregate_filter_clause database feature on SQLite >= 3.30

2019-10-10 Thread Django
#30859: Enable the supports_aggregate_filter_clause database feature on SQLite 
>=
3.30
-+-
 Reporter:  Simon Charette   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 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 Simon Charette):

 SQLite ticket https://www.sqlite.org/src/info/1079ad19993d13fa

-- 
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/067.7b4e66d534ceac0eaf0b8d9defa40054%40djangoproject.com.


Re: [Django] #30864: Document and endorse django.utils.decorators.classproperty (was: Please document and endorse django.utils.decorators.classproperty)

2019-10-10 Thread Django
#30864: Document and endorse django.utils.decorators.classproperty
-+-
 Reporter:  jgonggrijp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Documentation|  Version:  2.2
 Severity:  Normal   |   Resolution:
 Keywords:  utils classproperty  | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by Simon Charette):

 > Personally, I'd love having `cached_classproperty`…

 That would be hard to implement without some metaclass mockery or relying
 on some naive `if not hasattr(cls, '_cached_foo')` constructs.

 `cached_property` is efficient because it relies on the descriptor
 protocol to cache its return value into `__dict__` and avoid class level
 attribute lookups. We can't achieve similar things without defining the
 attribute on the class of the class (metaclass) and use the class's
 `__dict__` as a cache store.

 I guess the following could work.


 {{{#!python
 class cached_classproperty:
 NOT_SET = object()

 def __init__(self, method):
 self.method = method
 self.cached_value = self.NOT_SET

 def __get__(self, instance, owner):
 if self.cached_value is not self.NOT_SET:
 return self.cached_value

 self.cached_value = self.method(owner)
 return self.cached_value

  def __set__(self, instance, value):
  self.cached_value = value

  def __delete__(self, instance):
  self.cached_value = self.NOT_SET
 }}}

 But this wouldn't be as efficient as `cached_property` because `__get__`
 would always be called.

 Anyway the `cached_classproperty` case should probably be discussed in
 another ticket or on the mailing list.

-- 
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/068.0b552d90a7607ea951cf0ed3704801bc%40djangoproject.com.


Re: [Django] #30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py

2019-10-10 Thread Django
#30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py
--+
 Reporter:  Carlton Gibson|Owner:  (none)
 Type:  Cleanup/optimization  |   Status:  assigned
Component:  Error reporting   |  Version:  2.2
 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 Claude Paroz):

 * stage:  Unreviewed => Accepted


Comment:

 +1 to more precise testing!

-- 
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/071.978052b9f21b7a02319d24610ff1a249%40djangoproject.com.


Re: [Django] #30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py

2019-10-10 Thread Django
#30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py
-+-
 Reporter:  Carlton Gibson   |Owner:  (none)
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Error reporting  |  Version:  2.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Description changed by Carlton Gibson:

Old description:

> The tests in `tests/view_tests/tests/test_debug.py` are all (for want of
> a better phrase) ''integration level'' — they test whether particular
> values appear in the final output of either HTML, Plain Text, or either
> version of the email that's sent to admins.
>
> The trouble with this is that there's lot's of duplicate testing. As is
> inevitable, the tests are subtley out of sync. There are gaps in
> coverage, for instance in testing whether settings overrides actually
> work. And most, for me, looking at resolving the outstanding Error
> Reporting issues at the moment, it's **hard** to keep in mind exactly
> what tests should go where.
>
> As such, I'd like to refactor the tests. I'm just about to take that on,
> but I wanted to confirm that it would be OK before doing so. Coverage
> will be maintained. The tests will be clearer and just as effective, if
> not more so, and much easier to comprehend/maintain.
>
> General strategy, having looked at it:
>
> All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
> reduce all the ''Did the filtering work?'' tests down to that (or a
> single output format if testing the `technical_500` view).
>
> Then, the HTML or Plain text formats, use
> `ExceptionReporter.get_traceback_data()`. They in turn are used by the
> AdminEmailHandler. So not to loose the assurance that the final output is
> safe, I'd like to use mocks to assert that `get_traceback_data()` was
> actually employed. (Not usually a fan of mocking, but in this case it's
> right.)
>
> Additional tests can then focus on format specific questions, such as
> whether the correct format is used for Ajax requests, rather than
> repeating the filtering logic multiple times.
>
> As I say, primed to go but, wanting to confirm and record the intention.

New description:

 The tests in `tests/view_tests/tests/test_debug.py` are all (for want of a
 better phrase) ''integration level'' — they test whether particular values
 appear in the final output of either HTML, Plain Text, or either version
 of the email that's sent to admins.

 The trouble with this is that there's lot's of duplicate testing. As is
 inevitable, the tests are subtly out of sync. There are gaps in coverage,
 for instance in testing whether settings overrides actually work. And
 most, for me, looking at resolving the outstanding Error Reporting issues
 at the moment, it's **hard** to keep in mind exactly what tests should go
 where.

 As such, I'd like to refactor the tests. I'm just about to take that on,
 but I wanted to confirm that it would be OK before doing so. Coverage will
 be maintained. The tests will be clearer and just as effective, if not
 more so, and much easier to comprehend/maintain.

 General strategy, having looked at it:

 All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
 reduce all the ''Did the filtering work?'' tests down to that (or a single
 output format if testing the `technical_500` view).

 Then, the HTML or Plain text formats, use
 `ExceptionReporter.get_traceback_data()`. They in turn are used by the
 AdminEmailHandler. So not to loose the assurance that the final output is
 safe, I'd like to use mocks to assert that `get_traceback_data()` was
 actually employed. (Not usually a fan of mocking, but in this case it's
 right.)

 Additional tests can then focus on format specific questions, such as
 whether the correct format is used for Ajax requests, rather than
 repeating the filtering logic multiple times.

 As I say, primed to go but, wanting to confirm and record the intention,
 because it's a change in strategy.

--

-- 
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 
h

Re: [Django] #30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py

2019-10-10 Thread Django
#30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py
-+-
 Reporter:  Carlton Gibson   |Owner:  (none)
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Error reporting  |  Version:  2.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Description changed by Carlton Gibson:

Old description:

> The tests in `tests/view_tests/tests/test_debug.py` are all (for want of
> a better phrase) ''integration level'' — they test whether particular
> values appear in the final output of either HTML, Plain Text, or either
> version of the email that's sent to admins.
>
> The trouble with this is that there's lot's of duplicate testing. As is
> inevitable, the tests are subtley out of sync. There are gaps in
> coverage, for instance in testing whether settings overrides actually
> work. And most, for me, looking at resolving the outstanding Error
> Reporting issues at the moment, it's **hard** to keep in mind exactly
> what tests should go where.
>
> As such, I'd like to refactor the tests. I'm just about to take that on,
> but I wanted to confirm that it would be OK before doing so. Coverage
> will be maintained. The tests will be clearer and just as effective, if
> not more so, and much easier to comprehend/maintain.
>
> General strategy, having looked at it:
>
> All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
> reduce all the ''Did the filtering work?'' tests down to that (or a
> single output format if testing the `technical_500` view).
>
> Then, the HTML or Plain text formats, use
> `ExceptionReporter.get_traceback_data()`. They in turn are used by the
> AdminEmailHandler. So not to loose the assurance that the final output is
> safe, I'd like to use mocks to assert that `get_traceback_data()` was
> actually employed. (Not usually a fan of mocking, but in this case it's
> right.)
>
> Additional tests can then focus on format specific questions, such as
> whether the correct format is used for Ajax requests, rather than
> repeating the filtering logic multiple times.
>
> As I say, primed to go but, wanting to confirm and record the intention,
> as it's a change in strategy slightly.

New description:

 The tests in `tests/view_tests/tests/test_debug.py` are all (for want of a
 better phrase) ''integration level'' — they test whether particular values
 appear in the final output of either HTML, Plain Text, or either version
 of the email that's sent to admins.

 The trouble with this is that there's lot's of duplicate testing. As is
 inevitable, the tests are subtly out of sync. There are gaps in coverage,
 for instance in testing whether settings overrides actually work. And
 most, for me, looking at resolving the outstanding Error Reporting issues
 at the moment, it's **hard** to keep in mind exactly what tests should go
 where.

 As such, I'd like to refactor the tests. I'm just about to take that on,
 but I wanted to confirm that it would be OK before doing so. Coverage will
 be maintained. The tests will be clearer and just as effective, if not
 more so, and much easier to comprehend/maintain.

 General strategy, having looked at it:

 All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
 reduce all the ''Did the filtering work?'' tests down to that (or a single
 output format if testing the `technical_500` view).

 Then, the HTML or Plain text formats, use
 `ExceptionReporter.get_traceback_data()`. They in turn are used by the
 AdminEmailHandler. So not to loose the assurance that the final output is
 safe, I'd like to use mocks to assert that `get_traceback_data()` was
 actually employed. (Not usually a fan of mocking, but in this case it's
 right.)

 Additional tests can then focus on format specific questions, such as
 whether the correct format is used for Ajax requests, rather than
 repeating the filtering logic multiple times.

 As I say, primed to go but, wanting to confirm and record the intention,
 as it's a change in strategy slightly.

--

-- 
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

Re: [Django] #30864: Please document and endorse django.utils.decorators.classproperty

2019-10-10 Thread Django
#30864: Please document and endorse django.utils.decorators.classproperty
-+-
 Reporter:  jgonggrijp   |Owner:  nobody
 Type:  New feature  |   Status:  new
Component:  Documentation|  Version:  2.2
 Severity:  Normal   |   Resolution:
 Keywords:  utils classproperty  | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-

Comment (by Claude Paroz):

 Personally, I'd love having `cached_classproperty`…

-- 
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/068.3210e3f27ed2ae7f7843ff22fc49256a%40djangoproject.com.


Re: [Django] #30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py

2019-10-10 Thread Django
#30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py
-+-
 Reporter:  Carlton Gibson   |Owner:  (none)
 Type:   |   Status:  assigned
  Cleanup/optimization   |
Component:  Error reporting  |  Version:  2.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Description changed by Carlton Gibson:

Old description:

> The tests in `tests/view_tests/tests/test_debug.py` are all (for want of
> a better phrase) ''integration level'' — they test whether particular
> values appear in the final output of either HTML, Plain Text, or either
> version of the email that's sent to admins.
>
> The trouble with this is that there's lot's of duplicate testing. As is
> inevitable, the tests are subtley out of sync. There are gaps in
> coverage, for instance in testing whether settings overrides actually
> work. And most, for me, looking at resolving the outstanding Error
> Reporting issues at the moment, it's **hard** to keep in mind exactly
> what tests should go where.
>
> As such, I'd like to refactor the tests. I'm just about to take that on,
> but I wanted to confirm that it would be OK before doing so. Coverage
> will be maintained. The tests will be clearer and just as effective, if
> not more so, and much easier to comprehend/maintain.
>
> General strategy, having looked at it:
>
> All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
> reduce all the ''Did the filtering work?'' tests down to that (or a
> single output format if testing the `technical_500` view).
>
> Then, the HTML or Plain text formats, use
> `ExceptionReporter.get_traceback_data()`. They in turn are used by the
> AdminEmailHandler. So not to loose the assurance that the final output is
> safe, I'd like to use mocks to assert that `get_traceback_data()` was
> actually employed. (Not usually a fan of mocking, but in this case it's
> right.)
>
> Additional tests can then focus on format specific questions, such as
> whether the correct format is used for Ajax requests, rather than
> repeating the filtering logic multiple times.
>
> As I say, primed to go but, wanting to confirm and record the intention.

New description:

 The tests in `tests/view_tests/tests/test_debug.py` are all (for want of a
 better phrase) ''integration level'' — they test whether particular values
 appear in the final output of either HTML, Plain Text, or either version
 of the email that's sent to admins.

 The trouble with this is that there's lot's of duplicate testing. As is
 inevitable, the tests are subtley out of sync. There are gaps in coverage,
 for instance in testing whether settings overrides actually work. And
 most, for me, looking at resolving the outstanding Error Reporting issues
 at the moment, it's **hard** to keep in mind exactly what tests should go
 where.

 As such, I'd like to refactor the tests. I'm just about to take that on,
 but I wanted to confirm that it would be OK before doing so. Coverage will
 be maintained. The tests will be clearer and just as effective, if not
 more so, and much easier to comprehend/maintain.

 General strategy, having looked at it:

 All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
 reduce all the ''Did the filtering work?'' tests down to that (or a single
 output format if testing the `technical_500` view).

 Then, the HTML or Plain text formats, use
 `ExceptionReporter.get_traceback_data()`. They in turn are used by the
 AdminEmailHandler. So not to loose the assurance that the final output is
 safe, I'd like to use mocks to assert that `get_traceback_data()` was
 actually employed. (Not usually a fan of mocking, but in this case it's
 right.)

 Additional tests can then focus on format specific questions, such as
 whether the correct format is used for Ajax requests, rather than
 repeating the filtering logic multiple times.

 As I say, primed to go but, wanting to confirm and record the intention,
 as it's a change in strategy slightly.

--

-- 
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 vis

Re: [Django] #30849: Enabling hooking into the autodetector from a third-party app.

2019-10-10 Thread Django
#30849: Enabling hooking into the autodetector from a third-party app.
-+-
 Reporter:  Emil 'Skeen' Madsen  |Owner:  Emil
 |  'Skeen' Madsen
 Type:  New feature  |   Status:  closed
Component:  Migrations   |  Version:  2.2
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:  automigrator | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Emil 'Skeen' Madsen):

 Hi,

 Thanks for the feedback, the library is still in a very early stage, and I
 really dislike how the M object is implemented.
 If you happen to have a good idea for serializing a queryset, that would
 be lovely.

 Thanks for the links!

-- 
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/063.67ae7b36ec2aa22577c97edc0d2a03d5%40djangoproject.com.


[Django] #30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py

2019-10-10 Thread Django
#30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py
+--
   Reporter:  Carlton Gibson|  Owner:  (none)
   Type:  Cleanup/optimization  | Status:  assigned
  Component:  Error reporting   |Version:  2.2
   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 |
+--
 The tests in `tests/view_tests/tests/test_debug.py` are all (for want of a
 better phrase) ''integration level'' — they test whether particular values
 appear in the final output of either HTML, Plain Text, or either version
 of the email that's sent to admins.

 The trouble with this is that there's lot's of duplicate testing. As is
 inevitable, the tests are subtley out of sync. There are gaps in coverage,
 for instance in testing whether settings overrides actually work. And
 most, for me, looking at resolving the outstanding Error Reporting issues
 at the moment, it's **hard** to keep in mind exactly what tests should go
 where.

 As such, I'd like to refactor the tests. I'm just about to take that on,
 but I wanted to confirm that it would be OK before doing so. Coverage will
 be maintained. The tests will be clearer and just as effective, if not
 more so, and much easier to comprehend/maintain.

 General strategy, having looked at it:

 All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
 reduce all the ''Did the filtering work?'' tests down to that (or a single
 output format if testing the `technical_500` view).

 Then, the HTML or Plain text formats, use
 `ExceptionReporter.get_traceback_data()`. They in turn are used by the
 AdminEmailHandler. So not to loose the assurance that the final output is
 safe, I'd like to use mocks to assert that `get_traceback_data()` was
 actually employed. (Not usually a fan of mocking, but in this case it's
 right.)

 Additional tests can then focus on format specific questions, such as
 whether the correct format is used for Ajax requests, rather than
 repeating the filtering logic multiple times.

 As I say, primed to go but, wanting to confirm and record the intention.

-- 
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/056.3c7aa8409173aa81e3c22d2e5ba20ec8%40djangoproject.com.


Re: [Django] #30865: Document that some DATABASES['OPTIONS'] are not passed to dbshell database shell.

2019-10-10 Thread Django
#30865: Document that some DATABASES['OPTIONS'] are not passed to dbshell 
database
shell.
-+-
 Reporter:  Simon Charette   |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |
Component:  Documentation|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  dbshell database | Triage Stage:  Accepted
  options|
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  1|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * stage:  Unreviewed => Accepted


-- 
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/067.cb219d610d8ad7c775e8a1df09a19ae3%40djangoproject.com.


Re: [Django] #30849: Enabling hooking into the autodetector from a third-party app.

2019-10-10 Thread Django
#30849: Enabling hooking into the autodetector from a third-party app.
-+-
 Reporter:  Emil 'Skeen' Madsen  |Owner:  Emil
 |  'Skeen' Madsen
 Type:  New feature  |   Status:  closed
Component:  Migrations   |  Version:  2.2
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:  automigrator | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Simon Charette):

 > I have been able to implement my library via the new constraints API.
 Thank you

 You're welcome, thanks for giving it a try. It looks like this will likely
 be useful for a few people.

 If I can give you a suggestion regarding the implementation I'd avoid the
 `self.queryset == other.queryset` in `__eq__` as this will likely result
 in two queries during comparison which is quite frequent during auto-
 detection and operation reducing. Instead I'd compare each of the string
 representation of queries which is a documented API. That is
 `str(self.queryset.query) == str(other.queryset.query)`. I submitted
 [https://github.com/magenta-aps/django_queryset_constraint/pull/1 a PR]
 for it.

 >  I could conceivably similarly utilize the new constraints API to
 achieve the side goals I listed, such as:
 >
 > - Automatically enabling database extensions, when fields which require
 these extensions are utilized
 > - Add comments to database tables

 There's [https://github.com/django/django/pull/10540 an ongoing effort] to
 add `pre` and `post_operation` signals (#29843) for this purpose but in
 the team you can rely on `pre_migrate` to inject `CreateExtension` into
 the `plan` if a `CreateModel`, `AddField`, or `AlterField` has a field
 requiring an extension. Ditto with table comments.

-- 
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/063.b165fc70638736bad4e2fe9aa4806d88%40djangoproject.com.


Re: [Django] #56: Primary key columns should be UNSIGNED

2019-10-10 Thread Django
#56: Primary key columns should be UNSIGNED
-+-
 Reporter:  Manuzhai |Owner:  Caio
 |  Ariede
 Type:  New feature  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:  mysql| Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Caio Ariede):

 PR [https://github.com/django/django/pull/11900]

-- 
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/085.459bb494132d51a1d2a620eb0d388b36%40djangoproject.com.


Re: [Django] #30014: Initialising disabled ModelChoiceField yields 'Select a valid choice'-error despite initialised option being valid

2019-10-10 Thread Django
#30014: Initialising disabled ModelChoiceField yields 'Select a valid 
choice'-error
despite initialised option being valid
-+-
 Reporter:  thoha|Owner:  Etienne
 |  Chove
 Type:  Bug  |   Status:  assigned
Component:  Forms|  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  forms, disabled  | Triage Stage:  Accepted
  field, error, to_field_name|
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Etienne Chove):

 * version:  2.1 => master


-- 
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/063.05bc8acd2d394edcf098a43a180879ba%40djangoproject.com.


[Django] #30865: Document that some DATABASES['OPTIONS'] are not passed to dbshell database shell.

2019-10-10 Thread Django
#30865: Document that some DATABASES['OPTIONS'] are not passed to dbshell 
database
shell.
-+-
   Reporter:  Simon  |  Owner:  nobody
  Charette   |
   Type: | Status:  new
  Cleanup/optimization   |
  Component: |Version:  master
  Documentation  |   Keywords:  dbshell database
   Severity:  Normal |  options
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  1
  UI/UX:  0  |
-+-
 Some of `DATABASES['OPTIONS']` (e.g.
 [https://docs.djangoproject.com/en/2.2/ref/databases/#isolation-level
 isolation_level]) are not passed to the the underlying shell.

 In the long run I think we should try to add support for as many as of
 them as possible but since this will likely require and some time and
 discussion we should at least document the limitation.

 For context, we spent a good amount of trying to figure out why `SELECT
 @@TX_ISOLATION` was returning a `REPEATABLE-READ` while we had set
 `isolation_level = 'read committed'` in our `OPTIONS`.

 I think it might be more discoverable if the a note was added beside
 `DATABASES['OPTIONS']` instead of `dbshell`` as that's where we initially
 looked but that's just anecdotal evidence.

-- 
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/052.aa3a469bc7c88ee4e04232012cd38e6c%40djangoproject.com.


Re: [Django] #23755: patch_cache_control should special case "no-cache"

2019-10-10 Thread Django
#23755: patch_cache_control should special case "no-cache"
-+-
 Reporter:  thenewguy|Owner:  Flavio
 |  Curella
 Type:  New feature  |   Status:  assigned
Component:  Core (Cache system)  |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Mariusz Felisiak ):

 In [changeset:"9facc9002079d2713bb2591ca01f3aca0b678a00" 9facc900]:
 {{{
 #!CommitTicketReference repository=""
 revision="9facc9002079d2713bb2591ca01f3aca0b678a00"
 Refs #23755 -- Added tests for patch_cache_control() with no-cache Cache-
 Control directive.
 }}}

-- 
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/067.e1207dc42a81764728c8aa8bf90ce062%40djangoproject.com.


[Django] #30864: Please document and endorse django.utils.decorators.classproperty

2019-10-10 Thread Django
#30864: Please document and endorse django.utils.decorators.classproperty
-+-
   Reporter: |  Owner:  nobody
  jgonggrijp |
   Type:  New| Status:  new
  feature|
  Component: |Version:  2.2
  Documentation  |
   Severity:  Normal |   Keywords:  utils classproperty
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  1
  UI/UX:  0  |
-+-
 django.utils.decorators.classproperty is really useful. Please document
 it, encourage its use and make it a stable feature.

-- 
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/053.38f6be8dcfba09673a018ff29f2226e0%40djangoproject.com.


Re: [Django] #29241: ConditionalGetMiddleware and x-sendfile

2019-10-10 Thread Django
#29241: ConditionalGetMiddleware and x-sendfile
-+
 Reporter:  TZanke   |Owner:  nobody
 Type:  Bug  |   Status:  closed
Component:  Core (Cache system)  |  Version:  1.11
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:  sendfile | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+
Changes (by felixxm):

 * status:  new => closed
 * resolution:   => wontfix


Comment:

 Agreed, current behavior is RFC7232 compliant.

-- 
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/064.e94b7280b5774dfe7d2eb3283170b762%40djangoproject.com.


Re: [Django] #30849: Enabling hooking into the autodetector from a third-party app.

2019-10-10 Thread Django
#30849: Enabling hooking into the autodetector from a third-party app.
-+-
 Reporter:  Emil 'Skeen' Madsen  |Owner:  Emil
 |  'Skeen' Madsen
 Type:  New feature  |   Status:  closed
Component:  Migrations   |  Version:  2.2
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:  automigrator | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Emil 'Skeen' Madsen):

 Hello Simon,

 I have been able to implement my library via the new constraints API.
 Thank you for the hint.
 The library now lives at:
 * https://pypi.org/project/django-queryset-constraint/
 * https://github.com/magenta-aps/django_queryset_constraint

 

 I could conceivably similarly utilize the new constraints API to achieve
 the side goals I listed, such as:
 * Automatically enabling database extensions, when fields which require
 these extensions are utilized
 * Add comments to database tables

 However I believe this would be overloading the constraints API for
 something it was never designed for.
 So, do you have any ideas, as to how I would achieve these side goals
 without hooking into the autodetector and without utilizing the constraint
 API?
 If not, that is perfectly okay, these are, as stated, merely side goals.

-- 
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/063.0bc1cd6d9965d2f24b41709fe2bfbbf6%40djangoproject.com.


Re: [Django] #29241: ConditionalGetMiddleware and x-sendfile

2019-10-10 Thread Django
#29241: ConditionalGetMiddleware and x-sendfile
-+
 Reporter:  TZanke   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Core (Cache system)  |  Version:  1.11
 Severity:  Normal   |   Resolution:
 Keywords:  sendfile | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+

Comment (by Claude Paroz):

 Now that #30812 is fixed, I think this should be closed, as the behavior
 of returning a 304 if ETag is the same but Last-Modified has changed is
 conforming to the specs.
 See https://tools.ietf.org/html/rfc7232#section-6, `entity tags are
 presumed to be more accurate than date validators.`

-- 
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/064.2151a270846088b0563ed7e9a4857255%40djangoproject.com.


Re: [Django] #30812: ConditionalGetMiddleware returns 304 if ETag is the same but Last-Modified has changed.

2019-10-10 Thread Django
#30812: ConditionalGetMiddleware returns 304 if ETag is the same but 
Last-Modified
has changed.
--+
 Reporter:  Flavio Curella|Owner:  Dart
 Type:  Cleanup/optimization  |   Status:  closed
Component:  Core (Cache system)   |  Version:  master
 Severity:  Normal|   Resolution:  fixed
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by Mariusz Felisiak ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"ee6b17187fbf19d498c16bd46ec6dd6aaf86f453" ee6b1718]:
 {{{
 #!CommitTicketReference repository=""
 revision="ee6b17187fbf19d498c16bd46ec6dd6aaf86f453"
 Fixed #30812 -- Made ConditionalGetMiddleware set ETag only for responses
 with non-empty content.
 }}}

-- 
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/066.631b5ee0673005d347ddb6829fa1bb56%40djangoproject.com.


Re: [Django] #24686: Support for Moving a model between two Django apps

2019-10-10 Thread Django
#24686: Support for Moving a model between two Django apps
---+
 Reporter:  Alex Rothberg  |Owner:  nobody
 Type:  New feature|   Status:  new
Component:  Migrations |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  1
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+
Changes (by jedie):

 * cc: jedie (added)


-- 
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/067.282afe5dce2d4d35e9236d983c0c19c0%40djangoproject.com.