modelformset_factory and unique_together don't always validate unique fields

2014-07-02 Thread Jon Dufresne
I'm reporting this to the developers list as I feel this shows a
shortfall in (to me) expected behavior. I'm not sure this is exactly a
bug or simply a use case the unique validation wasn't designed for.

Basically, sometimes I want to create model formsets that use a
limited number of model fields. These model fields may have a unique
constraint using unique_together. The other field (or fields) of
unique together may not be included in the formset. Upon validating
the form, unique_together fields are only checked if all fields are
contained in the form. This means, my unique fields will not be
validated automatically. To achieve the validation I usually have to
copy parts of Django form validation, missing out on DRY.

I think one solution would be for model formsets to have a "static" or
"constant" fields argument. This could be a single value per field
that all forms in the formset would share for a particular set of
fields. These fields would not be a part of the rendered form, but
could be used for validating the forms and creating new models
instances. Thoughts?

An example where I might do this: suppose I have a "container" model.
This model has many "item" models created through a FK relationship.
Perhaps a field is unique together with the container. This example
could apply to any container/item relationship. I might create a
formset for all items of just one container for a bulk (unique)
rename. I have created a simple small example that illustrates my
point:

models.py


class Container(models.Model):
pass

class Item(models.Model):
container = models.ForeignKey(Container)
name = models.CharField(max_length=100)

class Meta:
unique_together = ('container', 'name')

ItemFormSet = modelformset_factory(model=Item, fields=['name'])


tests.py

class ItemFormSetTestCase(TestCase):
def test_unique_item_name(self):
container = Container()
container.save()
item1 = Item(container=container, name='item1')
item1.save()
item2 = Item(container=container, name='item2')
item2.save()
data = {
'form-TOTAL_FORMS': 2,
'form-INITIAL_FORMS': 2,
'form-MAX_NUM_FORMS': 2,
'form-0-id': str(item1.pk),
'form-0-name': 'newname',
'form-1-id': str(item2.pk),
'form-1-name': 'newname',
}
formset = ItemFormSet(
data,
queryset=Item.objects.filter(container=container))
self.assertFalse(formset.is_valid())
---

This test fails because the uniqueness of name is not actually
validated. If I were to go ahead an save this "valid" form, I receive
the following error:

---
Traceback (most recent call last):
  File "/home/jon/djtest/djtest/myapp/tests.py", line 27, in
test_unique_item_name
formset.save()
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py",
line 636, in save
return self.save_existing_objects(commit) + self.save_new_objects(commit)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py",
line 753, in save_existing_objects
saved_instances.append(self.save_existing(form, obj, commit=commit))
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py",
line 623, in save_existing
return form.save(commit=commit)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py",
line 457, in save
construct=False)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/forms/models.py",
line 103, in save_instance
instance.save()
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py",
line 590, in save
force_update=force_update, update_fields=update_fields)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py",
line 618, in save_base
updated = self._save_table(raw, cls, force_insert, force_update,
using, update_fields)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py",
line 680, in _save_table
forced_update)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/base.py",
line 724, in _do_update
return filtered._update(values) > 0
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/query.py",
line 598, in _update
return query.get_compiler(self.db).execute_sql(CURSOR)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/sql/compiler.py",
line 1003, in execute_sql
cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/models/sql/compiler.py",
line 785, in execute_sql
cursor.execute(sql, params)
  File 
"/home/jon/djtest/venv/lib/python2.7/site-packages/django/db/backends/utils.py",
line 65, in execute
return self.cursor.execute(sql, params)
  File 

Re: collectstatic - override default ignore list

2014-07-02 Thread Curtis Maloney
OT...just writing down this which came to me on the train this morning as I
read this thread


Does anyone else foresee the practice of having an "appconfig.py" full of
subclasses of AppConfigs becoming common?

Seems like a nice parallel to settings, allowing a project to specify
per-app config overrides in a central location.  And lets us dispense with
per-app settings with faux namespaces...

--
C



On 3 July 2014 07:17, Andre Terra  wrote:

>
> On Wed, Jul 2, 2014 at 5:14 PM, Tomas Ehrlich 
> wrote:
>
>> I would really like to see app configs with app-specific settings.
>> Personally, I don't like flat settings where namespaces are created
>> with prefixes (eg. STATICFILES_ ).
>>
>> The idea of overriding class attributes in AppConfig seems promising.
>>
>
> I wholeheartedly agree. Flat settings in faux namespaces seems very
> unpythonic to me. Even though it should prove slightly more cumbersome to
> have to edit settings in many different files during development, having
> the correct place for each setting makes for a much more organized
> structure.
>
> Forgive me if this question sounds silly, but how would settings for, say,
> staticfiles be accessed from third-party apps? Would we still use from
> django.conf import settings? If not, then how would we know to look for
> 'acme.apps.AcmeStaticFilesConfig' or 'staticfiles'?
>
> Also, shouldn't something like the ignore_patterns list be included in a
> wrapper settings attribute of StaticFilesConfig? This way we could easily
> access settings through
> apps.get_app_config('some_app').settings['ignore_patterns'].
>
> Based on Thomas' proposal:
>
>
> class Command(BaseCommand):
> # ...
> def set_options(self, **options):
> # ...
> staticfiles_config = apps.get_app_config('staticfiles')
>
>
> -   self.ignore_patterns = staticfiles_config.ignore_patterns
>
>
> +   self.ignore_patterns = staticfiles_config.settings['ignore_patterns']
>
>
>
> Sorry if this is completely wrong! I'm a long time lurker and hobbyist,
> but not a developer by occupation.
>
> Cheers,
> AT
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAKBiv3ygq-s7E2UkySW7ZEXEKZe5q1jOz9bhzy32ZCfc8316DQ%40mail.gmail.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>

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


Re: Regression in ValidationError in 1.7

2014-07-02 Thread Russell Keith-Magee
Hi Carl,

On Thu, Jul 3, 2014 at 5:33 AM, Carl Meyer  wrote:

> On 06/27/2014 06:41 AM, Russell Keith-Magee wrote:
> >
> > On Fri, Jun 27, 2014 at 7:35 PM, Curtis Maloney
> > > wrote:
> >
> > Am I reading this right as "people used to commonly solve this
> > problem by using an internal API, but now we have a public one...
> > AND the old internal API is now changed"?
> >
> > If so, the solution seems obvious -- document that it's time to move
> > the the official solution :)
> >
> >
> > Broadly speaking, I agree. However, my hesitation in categorically
> > agreeing stems from the fact that ValidationError *is* public API. Or,
> > at least, the constructor is, - but it seems a little absurd that we
> > formally document a way for people to construct an object, but not to
> > support any of the ways that they might have *used* that object.
>
> I don't think there's any absurdity. The documented and supported way to
> "use" ValidationError has always been to simply raise it from a clean()
> or clean_field() method, in which case Django itself catches it. Calling
> other undocumented methods on it it is clearly making use of unsupported
> internal API.
>

Sure - I can appreciate that argument. However, on the other side - the use
case I've described isn't completely left field, and a github code search
shows that update_error_dict() is being used in the wild for exactly that
reason.

That said - the discussion on the ticket and related PRs indicates that
restructuring necessary to make form.add_error() work mean that there
probably isn't a path that will allow backwards compatibility, so the
argument is moot at this point. The new add_error() API is much nicer, and
it isn't that hard to convert update_error_dict() usage into add_error()
usage, so I suppose we'll have to live with this upgrade wart.

Yours,
Russ Magee %-)

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


Re: Response Fixes

2014-07-02 Thread Łukasz Rekucki
On 2 July 2014 21:14, Aymeric Augustin
 wrote:
> 2014-07-02 15:36 GMT+02:00 Łukasz Rekucki :
>
>> It doesn't just alter it, but makes it conform to HTTP standard.
>
>
> As usual, given a different set of expectations, auto-fix is auto-break.
>
> We recently removed two of the four unconditional response "fixes".
>
> The remaining ones are:
>
> - `fix_location_header`: at least for some people, it's an issue. That's
>   why we're having a discussion.
>
> - `conditional_content_removal`: there's a variety of scenarios where
>   it fails. For instance, if you have a reverse proxy in front of your app
>   that performs gzipping, to handle HEAD requests correctly, it must
>   get the response body, compress it, figure out its length, and add it
>   to the Content-Length header.
>
> That's why I believe the concept of unconditional response "fixes" in
> itself is flawed.

I'm not saying it isn't, but sadly they are already there. I just
wanted to say, that I think this feature is not worth the potential
breakage. I'm sure there are better ways to achieve this without using
CGI or mod_wsgi.

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



-- 
Łukasz Rekucki

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


Re: Regression in ValidationError in 1.7

2014-07-02 Thread Carl Meyer
On 06/27/2014 06:41 AM, Russell Keith-Magee wrote:
> 
> On Fri, Jun 27, 2014 at 7:35 PM, Curtis Maloney
> > wrote:
> 
> Am I reading this right as "people used to commonly solve this
> problem by using an internal API, but now we have a public one...
> AND the old internal API is now changed"?
> 
> If so, the solution seems obvious -- document that it's time to move
> the the official solution :)
> 
> 
> Broadly speaking, I agree. However, my hesitation in categorically
> agreeing stems from the fact that ValidationError *is* public API. Or,
> at least, the constructor is, - but it seems a little absurd that we
> formally document a way for people to construct an object, but not to
> support any of the ways that they might have *used* that object.

I don't think there's any absurdity. The documented and supported way to
"use" ValidationError has always been to simply raise it from a clean()
or clean_field() method, in which case Django itself catches it. Calling
other undocumented methods on it it is clearly making use of unsupported
internal API.

I think Curtis is right, there is no problem with breaking use of the
undocumented API and documenting the new public API to achieve the same
thing.

Carl



signature.asc
Description: OpenPGP digital signature


Re: Response Fixes

2014-07-02 Thread Łukasz Rekucki
On 2 July 2014 15:42, Florian Apolloner  wrote:
> On Wednesday, July 2, 2014 3:36:22 PM UTC+2, Łukasz Rekucki wrote:
>>
>> It doesn't just alter it, but makes it conform to HTTP standard. While
>> most browsers will accept relative urls, I don't think Django should
>> sacrafice backwards compatibility for an arcane CGI feature.
>
>
> Internal redirects are imo an important feature to support, maybe not by
> default, but the response should be able to say "I know what I am doing".

Sure and that's why Nginx has X-Accel-Redirect. The problem with this
feature is that it requires using an (yet[1]) invalid value for an
already HTTP header. Personally, I would workaround this by adding my
own header like X-CGI-Forward and then make the CGI-Django wrapper
override Location with its value if it's present.

[1]: It seems the next version of HTTP 1.1 will allow non-absolute
URLs, but obviously CGI applications won't be able to return them to
the client because of this feature ;).

>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/19074151-e0eb-4925-90fe-9b1e5eedfd50%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.


-- 
Łukasz Rekucki

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


Re: collectstatic - override default ignore list

2014-07-02 Thread Andre Terra
On Wed, Jul 2, 2014 at 5:14 PM, Tomas Ehrlich 
wrote:

> I would really like to see app configs with app-specific settings.
> Personally, I don't like flat settings where namespaces are created
> with prefixes (eg. STATICFILES_ ).
>
> The idea of overriding class attributes in AppConfig seems promising.
>

I wholeheartedly agree. Flat settings in faux namespaces seems very
unpythonic to me. Even though it should prove slightly more cumbersome to
have to edit settings in many different files during development, having
the correct place for each setting makes for a much more organized
structure.

Forgive me if this question sounds silly, but how would settings for, say,
staticfiles be accessed from third-party apps? Would we still use from
django.conf import settings? If not, then how would we know to look for
'acme.apps.AcmeStaticFilesConfig' or 'staticfiles'?

Also, shouldn't something like the ignore_patterns list be included in a
wrapper settings attribute of StaticFilesConfig? This way we could easily
access settings through
apps.get_app_config('some_app').settings['ignore_patterns'].

Based on Thomas' proposal:


class Command(BaseCommand):
# ...
def set_options(self, **options):
# ...
staticfiles_config = apps.get_app_config('staticfiles')


-   self.ignore_patterns = staticfiles_config.ignore_patterns


+   self.ignore_patterns = staticfiles_config.settings['ignore_patterns']



Sorry if this is completely wrong! I'm a long time lurker and hobbyist, but
not a developer by occupation.

Cheers,
AT

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


Re: collectstatic - override default ignore list

2014-07-02 Thread Tomas Ehrlich
Dne Wed, 2 Jul 2014 21:17:52 +0200
Aymeric Augustin  napsal(a):

> 2014-07-02 17:21 GMT+02:00 Tomas Ehrlich :
> 
> > I'm just a bit confused with final decision.
> >
> 
> I don't think we have a decision yet ;-)

Tim Graham made comment on PR:
"Closing for now as the consensus seems to be adding a setting on the
AppConfig is the correct approach."

> 
> 
> > As Jannis commented on ticket: "This is simple, don't add a global setting
> > but a parameter to the staticfiles app config. That's where such things
> > belong now."
> >
> 
> That comment must have been written while I was working on app-loading and
> its final scope wasn't known yet.
> 
> I think a global setting isn't very different from a setting stored on the
> app config. The app config provides a bit of namespacing, but a prefix such
> as STATICFILES_ works just as well.
> 
> At this time, we don't handle settings for contrib apps in their app config
> classes. So if we want to handle this with a setting, it should be a global
> setting. If we don't, my explanations are irrelevant.
>

I would really like to see app configs with app-specific settings.
Personally, I don't like flat settings where namespaces are created
with prefixes (eg. STATICFILES_ ).

The idea of overriding class attributes in AppConfig seems promising.


Cheers,
   Tom

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


Re: collectstatic - override default ignore list

2014-07-02 Thread Jannis Leidel

On 02.07.2014, at 21:17, Aymeric Augustin  
wrote:

> 2014-07-02 17:21 GMT+02:00 Tomas Ehrlich :
> I'm just a bit confused with final decision.
> 
> I don't think we have a decision yet ;-) 
>  
> As Jannis commented on ticket: "This is simple, don't add a global setting
> but a parameter to the staticfiles app config. That's where such things
> belong now."
> 
> That comment must have been written while I was working on app-loading and 
> its final scope wasn't known yet.
> 
> I think a global setting isn't very different from a setting stored on the 
> app config. The app config provides a bit of namespacing, but a prefix such 
> as STATICFILES_ works just as well. 
> 
> At this time, we don't handle settings for contrib apps in their app config 
> classes. So if we want to handle this with a setting, it should be a global 
> setting. If we don't, my explanations are irrelevant.

Ugh, sorry for having created this confusion, I made the comment yesterday 
during my commute hence the brevity.

What I was thinking was indeed using a class attribute for such a configuration 
value because it could be overwritten on a per project basis if needed. I’m 
aware of that we have willfully punted on that last winter while discussing the 
app-loading refactor.

I was think something along the lines of this:

# django/contrib/staticfiles/apps.py
from django.apps import AppConfig

from django.utils.translation import ugettext_lazy as _


class StaticFilesConfig(AppConfig):
name = 'django.contrib.staticfiles'
verbose_name = _("Static Files")
ignore_patterns = ['CVS', '.*', '*~']  # <-- new line

# acme/apps.py
from django.contrib.staticfiles.apps import StaticFilesConfig

class AcmeStaticFilesConfig(StaticFilesConfig):
ignore_patterns = (StaticFilesConfig.ignore_patterns +
   [‘vendor/*', ‘endless_pit/*'])

# acme/settings.py

INSTALLED_APPS = [
'acme.apps.AcmeStaticFilesConfig',
]

# django/contrib/staticfiles/management/commands/collectstatic.py
from django.apps import apps

class Command(BaseCommand):
# ...
def set_options(self, **options):
# ...
staticfiles_config = apps.get_app_config('staticfiles')
self.ignore_patterns = staticfiles_config.ignore_patterns  <-- new line

I hope I’m not missing something. I guess this would only work once the app 
registry is populated, but isn’t that the case when the management command is 
called anyway?

Jannis



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: collectstatic - override default ignore list

2014-07-02 Thread Aymeric Augustin
2014-07-02 17:21 GMT+02:00 Tomas Ehrlich :

> I'm just a bit confused with final decision.
>

I don't think we have a decision yet ;-)


> As Jannis commented on ticket: "This is simple, don't add a global setting
> but a parameter to the staticfiles app config. That's where such things
> belong now."
>

That comment must have been written while I was working on app-loading and
its final scope wasn't known yet.

I think a global setting isn't very different from a setting stored on the
app config. The app config provides a bit of namespacing, but a prefix such
as STATICFILES_ works just as well.

At this time, we don't handle settings for contrib apps in their app config
classes. So if we want to handle this with a setting, it should be a global
setting. If we don't, my explanations are irrelevant.

-- 
Aymeric.

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


Re: Response Fixes

2014-07-02 Thread Aymeric Augustin
2014-07-02 15:36 GMT+02:00 Łukasz Rekucki :

It doesn't just alter it, but makes it conform to HTTP standard.


As usual, given a different set of expectations, auto-fix is auto-break.

We recently removed two of the four unconditional response "fixes".

The remaining ones are:

- `fix_location_header`: at least for some people, it's an issue. That's
  why we're having a discussion.

- `conditional_content_removal`: there's a variety of scenarios where
  it fails. For instance, if you have a reverse proxy in front of your app
  that performs gzipping, to handle HEAD requests correctly, it must
  get the response body, compress it, figure out its length, and add it
  to the Content-Length header.

That's why I believe the concept of unconditional response "fixes" in
itself is flawed.

-- 
Aymeric.

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


Re: collectstatic - override default ignore list

2014-07-02 Thread Tomas Ehrlich
Hi,
thank you for review.

I'm just a bit confused with final decision.

As Jannis commented on ticket: "This is simple, don't add a global setting 
but a parameter to the staticfiles app config. That's where such things 
belong now."

But according to Aymeric, app config isn't ready for custom settings yet -- 
"Maybe the answer is just a 'get_setting' function analogous to 'get_model'")


So the final conlusion is: New settings won't be added anywhere until this 
feature
will be implemented in AppConfig, right?


Thanks in advance

Cheers,
   Tom


Dne Wed, 2 Jul 2014 08:17:52 +0200
Aymeric Augustin  napsal(a):

> I've carefully deferred this topic until now...
> 
> Maybe the answer is just a "get_setting" function analogous to "get_model" :
> 
> apps.get_setting('myapp', 'MYSETTING')
> app_config.get_setting('MYSETTING')
> 
> It could simply defer to class level variables by default.
> 
> The first only works once the app registry is ready, while you may need 
> access to settings at import time. Technically we could make it work as soon 
> as the app import phase is done. Then I t would be available during the 
> module import phase. But I don't like basing an API on implementation 
> constraints.
> 
> The second one suffers from the same issue because one will obtain the 
> app_config through get_app_config which requires the app registry to be ready.
> 

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


Re: Response Fixes

2014-07-02 Thread Florian Apolloner
On Wednesday, July 2, 2014 3:36:22 PM UTC+2, Łukasz Rekucki wrote:
>
> It doesn't just alter it, but makes it conform to HTTP standard. While 
> most browsers will accept relative urls, I don't think Django should 
> sacrafice backwards compatibility for an arcane CGI feature. 


Internal redirects are imo an important feature to support, maybe not by 
default, but the response should be able to say "I know what I am doing".
 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/19074151-e0eb-4925-90fe-9b1e5eedfd50%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Response Fixes

2014-07-02 Thread Łukasz Rekucki
On Jul 2, 2014 2:09 PM, "Aymeric Augustin" <
aymeric.augus...@polytechnique.org> wrote:
>
> I find it wrong to alter the response created by the developer
unconditionally and not provide any escape hatch.

It doesn't just alter it, but makes it conform to HTTP standard. While most
browsers will accept relative urls, I don't think Django should sacrafice
backwards compatibility for an arcane CGI feature.

Therefore option 1 is my favorite. I'm proposing to documenting the
backwards incompatibility in the release notes.
>
> It will only affect project that do not use CommonMiddleware. There are
very few use cases for not using it. I expect that developers who made that
choice will know why and be able to adjust for the change.
>
> --
> Aymeric.
>
> 2014-07-02 3:11 GMT+02:00 peter :
>
>> This is in reference to this ticket:
https://code.djangoproject.com/ticket/17092
>>
>> There is a patch there to fix the specific use case of needing to
disable django's fix_location_header for certain responses in a CGI
compliant environment. Because of the way that response_fixes work you
can't just implement middleware to bypass them. So the patch in that ticket
adds an attribute to the response object that allows that 'fix' to be
bypassed when the handler processes the response. In that ticket aagustin
brought up that this might be better fixed by removing response_fixes from
the base handler and instead implementing them in common middleware making
it easier to add/remove or customize on a project by project basis rather
than adding attributes to the response object.
>>
>> Before going any further I wanted to see what the preferred approach
would be.
>>
>> I see 4 options.
>>
>> 1) Remove the concept of response_fixes from base handlers entirely and
re-implement what we currently have as middleware. This would break any
customized handlers that may be relying on  response_fixes as well as the
current redirect behavior for projects that aren't using common middleware.
>>
>> 2) Leave response_fixes as a feature of handlers but move the
fix_location_header functionality into middleware only breaking
applications that aren't using common middleware.
>>
>> 3) Apply the patch as is, address any future problems with
response_fixes as they come up.
>>
>> 4) Do nothing, projects that want the CGI compliant behavior can hack
django.
>>
>> --
>> You received this message because you are subscribed to the Google
Groups "Django developers" group.
>> To unsubscribe from this group and stop receiving emails from it, send
an email to django-developers+unsubscr...@googlegroups.com.
>> To post to this group, send email to django-developers@googlegroups.com.
>> Visit this group at http://groups.google.com/group/django-developers.
>> To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/d7b08325-6809-4b63-a948-4963bd590d5d%40googlegroups.com
.
>> For more options, visit https://groups.google.com/d/optout.
>
>
>
>
> --
> Aymeric.
>
> --
> You received this message because you are subscribed to the Google Groups
"Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/CANE-7mUjVR-uSZv-Du-p%3Dz%2BVF2LBdRKOjCEcX0_82qXe%3DWtzkg%40mail.gmail.com
.
>
> For more options, visit https://groups.google.com/d/optout.

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


Re: Response Fixes

2014-07-02 Thread Aymeric Augustin
I find it wrong to alter the response created by the developer
unconditionally and not provide any escape hatch. Therefore option 1 is my
favorite. I'm proposing to documenting the backwards incompatibility in the
release notes.

It will only affect project that do not use CommonMiddleware. There are
very few use cases for not using it. I expect that developers who made that
choice will know why and be able to adjust for the change.

-- 
Aymeric.

2014-07-02 3:11 GMT+02:00 peter :

> This is in reference to this ticket:
> https://code.djangoproject.com/ticket/17092
>
> There is a patch there to fix the specific use case of needing to disable
> django's fix_location_header for certain responses in a CGI compliant
> environment. Because of the way that response_fixes work you can't just
> implement middleware to bypass them. So the patch in that ticket adds an
> attribute to the response object that allows that 'fix' to be bypassed when
> the handler processes the response. In that ticket aagustin brought up that
> this might be better fixed by removing response_fixes from the base handler
> and instead implementing them in common middleware making it easier to
> add/remove or customize on a project by project basis rather than adding
> attributes to the response object.
>
> Before going any further I wanted to see what the preferred approach would
> be.
>
> I see 4 options.
>
> 1) Remove the concept of response_fixes from base handlers entirely and
> re-implement what we currently have as middleware. This would break any
> customized handlers that may be relying on  response_fixes as well as the
> current redirect behavior for projects that aren't using common middleware.
>
> 2) Leave response_fixes as a feature of handlers but move the
> fix_location_header functionality into middleware only breaking
> applications that aren't using common middleware.
>
> 3) Apply the patch as is, address any future problems with response_fixes
> as they come up.
>
> 4) Do nothing, projects that want the CGI compliant behavior can hack
> django.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/d7b08325-6809-4b63-a948-4963bd590d5d%40googlegroups.com
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>



-- 
Aymeric.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CANE-7mUjVR-uSZv-Du-p%3Dz%2BVF2LBdRKOjCEcX0_82qXe%3DWtzkg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Building a library of SQL functions into Django

2014-07-02 Thread Anssi Kääriäinen
On Wed, 2014-07-02 at 08:56 +0300, Anssi Kääriäinen wrote:
> The overhead is likely small, but it is hard to know without
> benchmarking. The WhereNode instances go through compiler.compile(), so
> generating SQL for qs.filter(*lots_of_args) should tell pretty well how
> much overhead there is.

The worst-case overhead of adding ops.compile_node(node, compiler) seems
to be around 5%. The test case is:

filter_clause = models.Q()
for i in range(0, 1):
filter_clause |= models.Q(id__exact=i)
qs = AModel.objects.filter(filter_clause)
for i in range(0, 100):
str(qs.query)

The slowdown seems to be almost the same even if you use just 10 Q()
objects instead of the non-realistic 1 Q() objects used above. The
reason is that when doing just str(qs.query) calls compiler.compile()
takes more than 60% of execution time even with 10 Q() objects. Main
portion of that execution time is spent in lookups.py as_sql(). Here is
top portion of the profile for 10 Q() objects, without
ops.compile_node() addition:

10.0070.0073.1663.166 tester.py:1()
 20000.0110.0002.6220.001 query.py:188(__str__)
 20000.0110.0002.6110.001 query.py:199(sql_with_params)
 20000.0750.0002.5520.001 compiler.py:85(as_sql)
48000/40000.1870.0002.1460.001 compiler.py:74(compile)
8000/40000.1670.0002.1220.001 where.py:85(as_sql)
20.1480.0001.7230.000 lookups.py:148(as_sql)
20.2670.0000.9420.000 lookups.py:138(process_lhs)
   410.0060.0000.6080.015 __init__.py:1()
20.1270.0000.5820.000 lookups.py:96(process_rhs)
10.0000.0000.5330.533 __init__.py:11(setup)
20.0810.0000.3600.000 lookups.py:87(get_db_prep_lookup)
20.0010.0010.3310.165 log.py:1()
20.0470.0000.3220.000 lookups.py:92(process_lhs)
30.0010.0000.3030.101 debug.py:1()
10.0000.0000.2780.278 urlresolvers.py:8()
20.1490.0000.2570.000 
__init__.py:655(get_db_prep_lookup)

The refactored lookup implementation is performing somewhat worse than
in 1.6, where compiler.py as_sql() takes only 1.996 seconds (2.552 for
1.7). We are just doing more in 1.7 than in 1.6, as the lookup's lhs is
query expression that needs to also go through compiler.compile().

There are some optimization points in 1.7 that could be used to remove
some of the overhead (process_rhs and get_db_prep_lookup coding
specifically). With some effort it is possible to get back to 1.6 speeds
if that is seen as important.

 - Anssi


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


Re: collectstatic - override default ignore list

2014-07-02 Thread Aymeric Augustin
I've carefully deferred this topic until now...

Maybe the answer is just a "get_setting" function analogous to "get_model" :

apps.get_setting('myapp', 'MYSETTING')
app_config.get_setting('MYSETTING')

It could simply defer to class level variables by default.

The first only works once the app registry is ready, while you may need access 
to settings at import time. Technically we could make it work as soon as the 
app import phase is done. Then I t would be available during the module import 
phase. But I don't like basing an API on implementation constraints.

The second one suffers from the same issue because one will obtain the 
app_config through get_app_config which requires the app registry to be ready.

-- 
Aymeric.

> Le 2 juil. 2014 à 04:08, Tim Graham  a écrit :
> 
> Jannis commented on the ticket, "This is simple, don't add a global setting 
> but a parameter to the staticfiles app config. That's where such things 
> belong now."
> 
> This makes sense to me. I suppose we should look into whether it makes sense 
> to move at least some of the other static files settings there as well. 
> Aymeric, is there any particular process that you envisioned for apps to add 
> arbitrary config options like this (besides the obvious way of class 
> attributes; and sorry if I've missed any existing docs about this).
> 
>> On Tuesday, July 1, 2014 6:48:08 PM UTC-4, Florian Apolloner wrote:
>> I see a point in ignoring hidden and swap files by default. CSV is imo 
>> something which shouldn't be used anymore and as such shouldn't be a 
>> default. Given that ignoring hidden and swap files should cover 80% of the 
>> use cases, I don't see much point in a new setting.
>> 
>> Cheers,
>> Florian
>> 
>>> On Tuesday, July 1, 2014 2:50:00 PM UTC+2, Tomáš Ehrlich wrote:
>>> Hi, 
>>> ticket #20189 proposes to override default ignore patterns for 
>>> collectstatic management command in project's settings. 
>>> 
>>> Patch has been submitted about a year ago, but right now it's waiting 
>>> for 'design decision' whether add a new setting or not. 
>>> 
>>> What is your opinion? 
>>> 
>>> Cheers, 
>>>Tom 
>>> 
>>> https://code.djangoproject.com/ticket/20189 
>>> https://github.com/django/django/pull/2854
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/44fbb093-1337-45e9-9445-3a8340454d12%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/AB7036D0-F8D1-4594-882B-A8B5FECD87C8%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.