from:"Gordon"

Re: Add verbosity option to manage.py checks that outputs which checks are ran

2020-06-12 Thread Gordon

The underlying problem that I want to solve is a way to fail a CI job if 
checks for an app don't run.  The only approach I currently see that would 
accomplish that is by using a custom tag for every project that could fail 
with the invalid tag error - but this feels wrong.

Would you be in favor of a flag for the check command that errors if checks 
aren't run from a particular namespace?

Like `python manage.py check --require-checks foo` which would exit with a 
non-zero exit code if zero checks for `foo` are run?



On Wednesday, June 10, 2020 at 6:40:16 PM UTC-4, Adam Johnson wrote:
>
> Perhaps there could be better on guidance in the documentation about 
> registering custom checks. I see nothing in the "writing your own checks" 
> guide really describes a recommended code structure. We'd accept a docs PR 
> there I'm sure.
>
> I normally have a checks submodule in my app and register the checks in 
> AppConfig.ready(). For example: 
> https://github.com/adamchainz/django-mysql/blob/2cf93c771f16011bc754a61e5aa95be4871f0363/src/django_mysql/apps.py#L17
>  
> . I don't like the decorator form since that's an import side effect and as 
> you've discovered it's not always easy to ensure your module gets imported.
>
> On Wed, 10 Jun 2020 at 21:16, Gordon > 
> wrote:
>
>> Particular case that spurred this - reusable app had checks to make sure 
>> urls were reversible.  Checks weren't imported.  Checks command passes but 
>> doesn't run the apps checks.  There is no indication that the checks were 
>> not run so you have to assume they did and were successful.
>>
>> The app checks were tested for function.  Since this imports the checks 
>> they would show as registered using your `get_checks()` test, no?  Seems 
>> kind of tricky...
>>
>> Since the checks framework is meant to be "extensible so you can easily 
>> add your own checks", it just seems like some sort of indication about 
>> which checks are run makes sense.  They are a great way for checking 
>> runtime conditions against production settings but not being able to 
>> confirm they actually ran somewhat negates the peace of mind that all is 
>> well.
>>
>> Thanks,
>> Gordon
>>
>>
>>
>> On Wednesday, June 10, 2020 at 11:59:39 AM UTC-4, Adam Johnson wrote:
>>>
>>> I am with Mariusz. Displaying the names of the check functions is a bit 
>>> against the intention of the checks framework. The check ID's are intended 
>>> to be enough information to reference the problem. This is different to 
>>> unit tests, where the function names are intended to carry information.
>>>
>>> Django doesn't really have a useful structure to its check function 
>>> names. For example a single check function, check_all_models() , is 
>>> responsible for all model level checks ( 
>>> https://github.com/django/django/blob/678c8dfee458cda77fce0d1c127f1939dc134584/django/core/checks/model_checks.py#L12
>>>  
>>> ). This has its own call tree through to Model.check() then each field's 
>>> Field.check(). Outputting a list entry like "check_all_models WARN" would 
>>> not give any actionable information.
>>>
>>> If you want to debug your own checks:
>>>
>>>1. Write tests for them. For example, use override_settings() in a 
>>>test case to set a setting to the value that would cause a warning, and 
>>>check that it returns the correct list of CheckMessage objects.
>>>2. You can inspect which check functions are registered with the 
>>>undocumented get_checks() function of the registry:
>>>
>>> In [1]: from django.core.checks.registry import registry
>>>
>>> In [2]: registry.get_checks()
>>> Out[2]:
>>> [>> **kwargs)>,
>>>  >> django.core.checks.urls.check_url_namespaces_unique(app_configs, **kwargs)>,
>>>  ...
>>>  >> **kwargs)>]
>>>
>>> If you're particularly unconfident that your checks.register() lines 
>>> don't always work, perhaps because they're conditional, you can also write 
>>> unit tests that run get_checks() and see your check functions are 
>>> registered.
>>>
>>> On Wed, 10 Jun 2020 at 15:00, Gordon  wrote:
>>>
>>>> This is a discussion for https://code.djangoproject.com/ticket/31681 - 
>>>> the feature would be useful to me and seems like something generally 
>>>> useful 
>>>> as well since it is implemented in the testing frame

Re: Add verbosity option to manage.py checks that outputs which checks are ran

2020-06-10 Thread Gordon

Particular case that spurred this - reusable app had checks to make sure 
urls were reversible.  Checks weren't imported.  Checks command passes but 
doesn't run the apps checks.  There is no indication that the checks were 
not run so you have to assume they did and were successful.

The app checks were tested for function.  Since this imports the checks 
they would show as registered using your `get_checks()` test, no?  Seems 
kind of tricky...

Since the checks framework is meant to be "extensible so you can easily add 
your own checks", it just seems like some sort of indication about which 
checks are run makes sense.  They are a great way for checking runtime 
conditions against production settings but not being able to confirm they 
actually ran somewhat negates the peace of mind that all is well.

Thanks,
Gordon



On Wednesday, June 10, 2020 at 11:59:39 AM UTC-4, Adam Johnson wrote:
>
> I am with Mariusz. Displaying the names of the check functions is a bit 
> against the intention of the checks framework. The check ID's are intended 
> to be enough information to reference the problem. This is different to 
> unit tests, where the function names are intended to carry information.
>
> Django doesn't really have a useful structure to its check function names. 
> For example a single check function, check_all_models() , is responsible 
> for all model level checks ( 
> https://github.com/django/django/blob/678c8dfee458cda77fce0d1c127f1939dc134584/django/core/checks/model_checks.py#L12
>  
> ). This has its own call tree through to Model.check() then each field's 
> Field.check(). Outputting a list entry like "check_all_models WARN" would 
> not give any actionable information.
>
> If you want to debug your own checks:
>
>1. Write tests for them. For example, use override_settings() in a 
>test case to set a setting to the value that would cause a warning, and 
>check that it returns the correct list of CheckMessage objects.
>2. You can inspect which check functions are registered with the 
>undocumented get_checks() function of the registry:
>
> In [1]: from django.core.checks.registry import registry
>
> In [2]: registry.get_checks()
> Out[2]:
> [ **kwargs)>,
>   django.core.checks.urls.check_url_namespaces_unique(app_configs, **kwargs)>,
>  ...
>   **kwargs)>]
>
> If you're particularly unconfident that your checks.register() lines don't 
> always work, perhaps because they're conditional, you can also write unit 
> tests that run get_checks() and see your check functions are registered.
>
> On Wed, 10 Jun 2020 at 15:00, Gordon > 
> wrote:
>
>> This is a discussion for https://code.djangoproject.com/ticket/31681 - 
>> the feature would be useful to me and seems like something generally useful 
>> as well since it is implemented in the testing framework.  The closing 
>> comment mentions that copying a documentation page into command output 
>> isn't valuable.  That isn't what I meant to suggest so I will attempt to 
>> explain more clearly below:
>>
>> It would be helpful if `checks` would output what checks it is running.  
>> Currently you don't actually know if a check runs or not.  Perhaps it isn't 
>> registered correctly?  Or it was excluded by mistake when you expected it 
>> to run?  I like to keep this information around in build history for quick 
>> inspection purposes as needed.
>>
>>
>> For example, when running a check it would be nice to (optionally) see 
>> something of the form:
>>
>>- path.to.my.checkA ... ok
>>- path.to.my.checkB ... ERROR
>>- path.to.my.checkC ... ok
>>- path.to.my.checkD ... WARN
>>
>> System check ran 100 checks and identified 2 issues (0 silenced).
>>
>>  
>>
>> It would be helpful if the `checks` verbosity flag worked in similar 
>> fashion to the testing framework.  I am thinking that `-v 1` or unspecified 
>> would maintain the current behavior.  `-v 2` outputs all checks run not 
>> under the django namespace (due to the comment in the ticket about not 
>> repeating a documentation page).  And `-v 3` outputs all checks.  It might 
>> also be helpful to add a verbosity level that only prints checks under the 
>> current working directory (i.e. project specific checks) but this might be 
>> difficult to determine correctly based on project layout and installation 
>> method.
>>
>> If the general consensus likes the feature and the issue can be 
>> re-opened, I will investigate.  The ticket says it would not be feasible to 
>> output this information.  I am assuming (since they are run

Add verbosity option to manage.py checks that outputs which checks are ran

2020-06-10 Thread Gordon

This is a discussion for https://code.djangoproject.com/ticket/31681 - the
feature would be useful to me and seems like something generally useful as
well since it is implemented in the testing framework. The closing comment
mentions that copying a documentation page into command output isn't
valuable. That isn't what I meant to suggest so I will attempt to explain
more clearly below:

It would be helpful if `checks` would output what checks it is running.
Currently you don't actually know if a check runs or not. Perhaps it isn't
registered correctly? Or it was excluded by mistake when you expected it
to run? I like to keep this information around in build history for quick
inspection purposes as needed.

For example, when running a check it would be nice to (optionally) see
something of the form:

- path.to.my.checkA ... ok
- path.to.my.checkB ... ERROR
- path.to.my.checkC ... ok
- path.to.my.checkD ... WARN

System check ran 100 checks and identified 2 issues (0 silenced).

It would be helpful if the `checks` verbosity flag worked in similar
fashion to the testing framework. I am thinking that `-v 1` or unspecified
would maintain the current behavior. `-v 2` outputs all checks run not
under the django namespace (due to the comment in the ticket about not
repeating a documentation page). And `-v 3` outputs all checks. It might
also be helpful to add a verbosity level that only prints checks under the
current working directory (i.e. project specific checks) but this might be
difficult to determine correctly based on project layout and installation
method.

If the general consensus likes the feature and the issue can be re-opened,
I will investigate. The ticket says it would not be feasible to output
this information. I am assuming (since they are run generically via a
registration approach) that it should be straightforward to log the dotted
path of the check.

Please let me know what you think!

Re: handle_uncaught_exception error logging

2020-04-12 Thread Gordon

Hrm.  Thanks for the link... probably not going to end up being a Django 
issue.  I will report back if it is.  

On Sunday, April 12, 2020 at 10:25:07 AM UTC-4, Tim Graham wrote:
>
> The relevant commit might be 
> https://github.com/django/django/commit/10b44e45256ddda4258ae032b8d4725a3e3284e6.
>  
> That change was made in Django 2.1. You didn't say what version of Django 
> you're using.
>
> You'll have to give more details about your case and what's behaving 
> differently.
>
> On Sunday, April 12, 2020 at 8:57:12 AM UTC-4, Gordon wrote:
>>
>> Why was the error log removed from handle_uncaught_exception?  I didn't 
>> find mention about it but I've recently encountered a middleware causing a 
>> 500 error and not getting a traceback log was unexpected.
>>
>>
>> https://github.com/django/django/blob/stable/2.0.x/django/core/handlers/exception.py#L115
>>
>> Should it be added back or is it intentionally removed?
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/26ae2557-0035-4a26-99a6-c84d010604ee%40googlegroups.com.

handle_uncaught_exception error logging

2020-04-12 Thread Gordon

Why was the error log removed from handle_uncaught_exception?  I didn't 
find mention about it but I've recently encountered a middleware causing a 
500 error and not getting a traceback log was unexpected.

https://github.com/django/django/blob/stable/2.0.x/django/core/handlers/exception.py#L115

Should it be added back or is it intentionally removed?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/596984fa-8d98-4abf-b8f3-128a34e2025d%40googlegroups.com.

Re: Management command that fails when there are created migrations waiting to be applied to the database

2020-03-26 Thread Gordon

Exactly!  I need to know if we will *run* migrations.

On Thursday, March 26, 2020 at 2:59:39 AM UTC-4, Fran Hrženjak wrote:
>
> If I’m understanding correctly, the difference between `makemigrations 
> --check` and your use case is that one checks if we need to *create* any 
> migrations while the other checks (or would check) if we need to *run* any 
> migrations.
>
> Seems reasonable to me. Although I would prefer we just add a `--check` 
> option to the existing `migrate` command to keep it simpler and consistent 
> with the existing option for `makemigrations` command. 
>
>  
>
> On Thu, 26 Mar 2020 at 07:29, Gordon > 
> wrote:
>
>> Good morning.
>>
>> I had created ticket https://code.djangoproject.com/ticket/31402 after 
>> discussing in IRC about a built in way to stop a continuous delivery 
>> pipeline if the deployment would need to run database migrations. (i.e. if 
>> running `python manage.py migration` would do anything or not.
>>
>> In my specific scenario, I would like to run `python manage.py 
>> showmigrations --database=foo --check` and have it fail if there are 
>> unapplied migrations to the 'foo' database (or the default database as the 
>> case may be).  In this case, the automatic deployment would halt to prevent 
>> downtime.
>>
>> The discussion resulted with one must parse the results from `python 
>> manage.py showmigrations`.  I.e. write a script that checks the output of 
>> `showmigrations` for something like '[ ]'.
>>
>> I can certainly do that, but it seems like it would be generally useful 
>> to have this type of command available within Django.  It could be used to 
>> improve the release process for any project that uses some sort of 
>> automatic deployment scenario.
>>
>> The ticket was rejected with the response, use `python manage.py 
>> makemigrations --check`.  However, I believe that this command determines 
>> if the current version of models on disk would result in a new migration 
>> being made.  This is not the same as the functionality I have suggested.  
>> Am I wrong here?
>>
>> Please let me know your thoughts.
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to django-d...@googlegroups.com .
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msgid/django-developers/728bde3f-e68c-4224-9101-5c98a45c1016%40googlegroups.com
>>  
>> <https://groups.google.com/d/msgid/django-developers/728bde3f-e68c-4224-9101-5c98a45c1016%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>>
> -- 
> LP,
> Fran
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/b9e04f84-2dd7-4fd7-96a8-35d3a87aea12%40googlegroups.com.

Management command that fails when there are created migrations waiting to be applied to the database

2020-03-25 Thread Gordon

Good morning.

I had created ticket https://code.djangoproject.com/ticket/31402 after
discussing in IRC about a built in way to stop a continuous delivery
pipeline if the deployment would need to run database migrations. (i.e. if
running `python manage.py migration` would do anything or not.

In my specific scenario, I would like to run `python manage.py
showmigrations --database=foo --check` and have it fail if there are
unapplied migrations to the 'foo' database (or the default database as the
case may be). In this case, the automatic deployment would halt to prevent
downtime.

The discussion resulted with one must parse the results from `python
manage.py showmigrations`. I.e. write a script that checks the output of
`showmigrations` for something like '[ ]'.

I can certainly do that, but it seems like it would be generally useful to
have this type of command available within Django. It could be used to
improve the release process for any project that uses some sort of
automatic deployment scenario.

The ticket was rejected with the response, use `python manage.py
makemigrations --check`. However, I believe that this command determines
if the current version of models on disk would result in a new migration
being made. This is not the same as the functionality I have suggested.
Am I wrong here?

Please let me know your thoughts.

Re: Automatic prefetching in querysets

2020-03-25 Thread Gordon Wrigley

My existing code for this is now available as a pypi package
https://github.com/tolomea/django-auto-prefetch 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/aaf633e3-a099-4cb4-aad7-10cfe6b11596%40googlegroups.com.

Re: Add subdomains of localhost to ALLOWED_HOSTS in DEBUG mode

2019-11-21 Thread gordon

Spec:

   - https://tools.ietf.org/html/rfc6761#section-6.3

It didn't seem to work on IE edge.  At the rate of browser updates I expect
all browsers to support this by 2090.  But it does make development on
Chrome nice now =)

On Thu, Nov 21, 2019 at 4:49 PM Adam Johnson  wrote:

> I’m all in favour of making development easier. Would be in favour of this
> if I could see a source :) Also can you check the behaviour in more
> browsers than Chrom(e|ium)? :)
>
> On Thu, 21 Nov 2019 at 21:40, Gordon  wrote:
>
>> Good afternoon,
>>
>> It seems pretty straightforward to me as a win with no downsides.  Chrome
>> resolves subdomains of localhost as localhost.  I did a little searching
>> (but forgot to keep the sources) and discovered that this behavior was
>> intentional and spec compliant.  This is extremely handy for local
>> development since it allows one to test subdomains without fooling with
>> network DNS or modifying host files.
>>
>> I opened ticket https://code.djangoproject.com/ticket/31010 and was
>> asked to canvas for opinions.
>>
>> I think that updating the handling of ALLOWED_HOSTS in
>> HttpRequest.get_host()from:
>>
>> # Allow variants of localhost if ALLOWED_HOSTS is empty and 
>> DEBUG=True.
>> allowed_hosts = settings.ALLOWED_HOSTS
>> if settings.DEBUG and not allowed_hosts:
>> allowed_hosts = ['localhost', '127.0.0.1', '[::1]']
>>
>>
>> to
>>
>> # Allow variants of localhost if ALLOWED_HOSTS is empty and 
>> DEBUG=True.
>> allowed_hosts = settings.ALLOWED_HOSTS
>> if settings.DEBUG and not allowed_hosts:
>> allowed_hosts = ['.localhost', '127.0.0.1', '[::1]']
>>
>>
>> would be safe and support this pattern without requiring any changes to
>> settings files which is quite nice for new projects.  Please let me know
>> what you think =)
>>
>> Thanks,
>> Gordon
>>
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/b9e276f0-76f1-4a9c-be22-cb7286c20ec5%40googlegroups.com
>> <https://groups.google.com/d/msgid/django-developers/b9e276f0-76f1-4a9c-be22-cb7286c20ec5%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>>
> --
> Adam
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/xcoAF9Gm_dI/unsubscribe
> .
> To unsubscribe from this group and all its topics, send an email to
> django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAMyDDM0ufpyu50t_tbACPrEwJ94LYTWGyfywGzooA7%3D3pVnXBw%40mail.gmail.com
> <https://groups.google.com/d/msgid/django-developers/CAMyDDM0ufpyu50t_tbACPrEwJ94LYTWGyfywGzooA7%3D3pVnXBw%40mail.gmail.com?utm_medium=email&utm_source=footer>
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAJc1Fha6-BVPtehCQQXjZaAdbeqhoUzxLVkW-06fnZD1VWfyEg%40mail.gmail.com.

Add subdomains of localhost to ALLOWED_HOSTS in DEBUG mode

2019-11-21 Thread Gordon

Good afternoon,

It seems pretty straightforward to me as a win with no downsides. Chrome
resolves subdomains of localhost as localhost. I did a little searching
(but forgot to keep the sources) and discovered that this behavior was
intentional and spec compliant. This is extremely handy for local
development since it allows one to test subdomains without fooling with
network DNS or modifying host files.

I opened ticket https://code.djangoproject.com/ticket/31010 and was asked
to canvas for opinions.

I think that updating the handling of ALLOWED_HOSTS in
HttpRequest.get_host()from:

# Allow variants of localhost if ALLOWED_HOSTS is empty and DEBUG=True.
allowed_hosts = settings.ALLOWED_HOSTS
if settings.DEBUG and not allowed_hosts:
allowed_hosts = ['localhost', '127.0.0.1', '[::1]']

# Allow variants of localhost if ALLOWED_HOSTS is empty and DEBUG=True.
allowed_hosts = settings.ALLOWED_HOSTS
if settings.DEBUG and not allowed_hosts:
allowed_hosts = ['.localhost', '127.0.0.1', '[::1]']

would be safe and support this pattern without requiring any changes to
settings files which is quite nice for new projects. Please let me know
what you think =)

Thanks,
Gordon

Re: Force "required" fields to be included in a ModelForm

2019-04-17 Thread Will Gordon

Well shoot...this definitely seems like something only I'm running into. 
Appreciate y'alls feedback. Thanks 👍🏻

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

Re: Force "required" fields to be included in a ModelForm

2019-04-16 Thread Will Gordon

Would it be weird to just make it so that the "required" field *must* be 
present in exclude? This way, if you *accidentally* leave off a required 
field, you're quickly notified about itbut if you explicitly mark it as 
something to exclude, it makes it clear to everyone exactly what you're 
trying to do? The whole "explicit is better than implicit" and what not.

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

Re: Force "required" fields to be included in a ModelForm

2019-04-16 Thread Will Gordon

I can certainly agree that there may be some use cases where it should be 
possible to disable this functionality, but I would argue that erroring on 
a missing, required field should be the default, and allow for a way to 
override...as opposed to allowing missing fields and requiring a workaround 
to ensure they're not missing.

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

Re: Force "required" fields to be included in a ModelForm

2019-04-16 Thread Will Gordon

Sorry if I wasn't more clear, but using a warning was exactly what I was 
proposing. In the same way that a FieldError is raised when an editable field 
is listed in fields, I was essentially planning on doing the exact same 
check on the blank attribute. I agree that this should be ignorable, and 
I'm not sure that I've settled on a way to do that as of yet.

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

Force "required" fields to be included in a ModelForm

2019-04-16 Thread Will Gordon

In the same way that editable fields are forced to not be included in a 
ModelForm (
https://github.com/django/django/blob/master/django/forms/models.py#L146), 
I would like to propose that "required" fields (`blank=False`) be forced to 
be included in a ModelForm.

While I understand that a developer can force this inclusion themselves, 
but on a large project, it should not be necessary to always ensure that a 
Model and ModelForm are in sync.

Since this is probably a non-trivial patch (
https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#non-trivial-patches)
 
I need to provide evidence that this has been discussed. As such, I'm open 
to any and all opinions!

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

Re: Why does ModelForm do validation and not Model

2019-04-16 Thread Will Gordon

Ahh, cool. That makes more sense. I worry that it still leaves open the 
potential of accidentally not validating something. It may make more sense 
to offer instance.save(validate=False) instead of relying on the developer 
to always know whether they can trust the input. But I agree that for 
historical reasons, I'm sure it's much more difficult to implement this now.

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

Re: Why does ModelForm do validation and not Model

2019-04-16 Thread Will Gordon

So the validation is cheaper when performed by ModelForm, as opposed to the 
Model?

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

Why does ModelForm do validation and not Model

2019-04-16 Thread Will Gordon

I can't seem to find a good reason for this. And I could foresee this 
preventing potential mistakes. I'm not proposing to actually change the 
implementation, I guess I'm just looking for the reason of it.

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

Re: Sealing or locking QuerySets -- a suggestion to prevent surprise N+1 queries.

2018-01-08 Thread Gordon Wrigley

Regarding auto prefetch, after the discussion here I created a 
ticket https://code.djangoproject.com/ticket/28586
It didn't get much attention presumably because it was around Django 2 
release time.
I have a todo note to add tests and doco and generally get it to a mergable 
state in order to push the conversation along.
Also the pull as it stands has a little bug I have to upload the fix for.

On Friday, January 5, 2018 at 2:41:49 AM UTC, Josh Smeaton wrote:
>
> I wasn't aware of this new feature Shai, thanks for pointing it out!
>
> For this particular case I'd prefer locking to be bound to a particular 
> queryset rather than the database as a whole. I would also expect it to 
> fail loudly when accessing a non-fetched related object (book.author), 
> which can be a common cause of pain.
>
> I'm also still very interested in auto-prefetch Adam, is there any help I 
> can lend?
>
> On Thursday, 4 January 2018 17:32:19 UTC+11, Shai Berger wrote:
>>
>> Hi all, 
>>
>> Django 2.0 has a new feature[1] which allows you to say "I expect no 
>> actual 
>> database queries in this piece of code". It is not designed to stop a 
>> specific 
>> queryset from spawning requests, so getting it to do exactly what's asked 
>> for 
>> in this thread may be a little involved, but if your goal is to prevent 
>> surprise queries, I think it is actually better than sealing a single 
>> queryset. 
>>
>> HTH, 
>> Shai 
>>
>> [1] https://docs.djangoproject.com/en/2.0/topics/db/instrumentation/ 
>>
>

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

Re: Automatic prefetching in querysets

2017-09-12 Thread Gordon Wrigley

This received some positive responses, so to help move the conversation
along I have created a ticket and pull request.

https://code.djangoproject.com/ticket/28586
https://github.com/django/django/pull/9064

Regards G

On Tue, Aug 15, 2017 at 10:44 AM, Gordon Wrigley 
wrote:

> I'd like to discuss automatic prefetching in querysets. Specifically
> automatically doing prefetch_related where needed without the user having
> to request it.
>
> For context consider these three snippets using the Question & Choice
> models from the tutorial
> <https://docs.djangoproject.com/en/1.11/intro/tutorial02/#creating-models> 
> when
> there are 100 questions each with 5 choices for a total of 500 choices.
>
> Default
> for choice in Choice.objects.all():
> print(choice.question.question_text, ':', choice.choice_text)
> 501 db queries, fetches 500 choice rows and 500 question rows from the DB
>
> Prefetch_related
> for choice in Choice.objects.prefetch_related('question'):
> print(choice.question.question_text, ':', choice.choice_text)
> 2 db queries, fetches 500 choice rows and 100 question rows from the DB
>
> Select_related
> for choice in Choice.objects.select_related('question'):
> print(choice.question.question_text, ':', choice.choice_text)
> 1 db query, fetches 500 choice rows and 500 question rows from the DB
>
> I've included select_related for completeness, I'm not going to propose
> changing anything about it's use. There are places where it is the best
> choice and in those places it will still be up to the user to request it. I
> will note that anywhere select_related is optimal prefetch_related is still
> better than the default and leave it at that.
>
> The 'Default' example above is a classic example of the N+1 query problem,
> a problem that is widespread in Django apps.
> This pattern of queries is what new users produce because they don't know
> enough about the database and / or ORM to do otherwise.
> Experieced users will also often produce this because it's not always
> obvious what fields will and won't be used and subsequently what should be
> prefetched.
> Additionally that list will change over time. A small change to a template
> to display an extra field can result in a denial of service on your DB due
> to a missing prefetch.
> Identifying missing prefetches is fiddly, time consuming and error prone.
> Tools like django-perf-rec <https://github.com/YPlan/django-perf-rec>
> (which I was involved in creating) and nplusone
> <https://github.com/jmcarp/nplusone> exist in part to flag missing
> prefetches introduced by changed code.
> Finally libraries like Django Rest Framework and the Admin will also
> produce queries like this because it's very difficult for them to know what
> needs prefetching without being explicitly told by an experienced user.
>
> As hinted at the top I'd like to propose changing Django so the default
> code behaves like the prefetch_related code.
> Longer term I think this should be the default behaviour but obviously it
> needs to be proved first so for now I'd suggest a new queryset function
> that enables this behaviour.
>
> I have a proof of concept of this mechanism that I've used successfully in
> production. I'm not posting it yet because I'd like to focus on desired
> behavior rather than implementation details. But in summary, what it does
> is when accessing a missing field on a model, rather than fetching it just
> for that instance, it runs a prefetch_related query to fetch it for all
> peer instances that were fetched in the same queryset. So in the example
> above it prefetches all Questions in one query.
>
> This might seem like a risky thing to do but I'd argue that it really
> isn't.
> The only time this isn't superior to the default case is when you are post
> filtering the queryset results in Python.
> Even in that case it's only inferior if you started with a large number of
> results, filtered basically all of them and the code is structured so that
> the filtered ones aren't garbage collected.
> To cover this rare case the automatic prefetching can easily be disabled
> on a per queryset or per object basis. Leaving us with a rare downside that
> can easily be manually resolved in exchange for a significant general
> improvement.
>
> In practice this thing is almost magical to work with. Unless you already
> have extensive and tightly maintained prefetches everywhere you get an
> immediate boost to virtually everything that touches the database, often
> knocking orders of magnitude off page load times.
>
> If an agreement ca

Re: Automatic prefetching in querysets

2017-08-16 Thread Gordon Wrigley

I'm not advocating either way on this, but it's probably worth noting that
the context manager proposal is compatible with a queryset level optin/out
and an object level optout.

So you could for example have prefetch_related(auto=None) which can be
explicitly set to True / False and takes it's value from the context
manager when it's None.


On Thu, Aug 17, 2017 at 12:42 AM, Josh Smeaton 
wrote:

> I think there's a lot right with your suggestions here Shai.
>
> It delivers better default behaviour for new projects, does not affect
> existing deployments, and seems pretty easy to enable/disable selectively
> at any level of the stack.
>
> My only concern would be libraries leaning on this behaviour by enabling
> it locally and users being unable to change it. That's only a small concern
> though, and wouldn't prevent me from recommending the proposal.
>
>
> On Thursday, 17 August 2017 09:34:03 UTC+10, Shai Berger wrote:
>>
>> First of all, I think making the auto-prefetch feature available in some
>> form
>> is a great idea. As an opt-in, I would have made use of it on several
>> occasions, and cut days of optimization work to minutes. It's true that
>> this
>> wouldn't achieve the best possible optimization in many cases, but it
>> would be
>> close enough, for a small fraction of the effort.
>>
>> But of the choices that have been suggested here, I find none quite
>> satisfactory. I agree, basically, with almost all of the objections
>> raised on
>> all sides. And I think I have a better suggestion.
>>
>> Rather than a setting (global and essentially constant) or all-local
>> opt-ins,
>> suppose the feature was controlled -- for all querysets -- by a context
>> manager.
>>
>> So, rather than
>>
>> > "qs.prefetch_related(auto=True)", or "with qs.auto_prefetch():",
>>
>> suppose we had
>>
>> with QuerySet.auto_prefetch(active=True):
>>
>> and under that, all querysets would auto-prefetch. Then:
>>
>> - We could put that in a middleware; using that middleware would then,
>> effectively, be a global opt in. We could add the middleware -- perhaps
>> commented-out -- to the new project template, for beginner's sake. Call
>> it
>> PerformanceTrainingWheelsMiddleware, to make people aware that it's
>> something
>> they should grow out of, but make it easily accessible.
>>
>> - We could easily build a decorator for views
>>
>> - We could have local opt-out, at the correct level -- not the objects,
>> but
>> the control flow.
>>
>> For this to work, the flag controlling the feature would need to be
>> thread-
>> local and managed as a stack. But these aren't problems.
>>
>> If (as likely) this suggestion still does not generate a concensus, I
>> would
>> prefer that we follow the path Anssi suggested --
>>
>> > Note that it should be possible to implement this fully as 3rd party
>> > project if we add an easy way to customise how related object fetching
>> is
>> > done. I'm not sure if we can add an easy way for that.
>>
>> Except I don't think "easy" is a requirement here. If we can add a
>> sensible
>> way, that would be enough -- I don't expect many Django users to develop
>> implementations of the feature, only to use them.
>>
>> My 2 cents,
>> Shai.
>>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit https://groups.google.com/d/
> topic/django-developers/EplZGj-ejvg/unsubscribe.
> To unsubscribe from this group and all its topics, 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 https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/bf3e22af-9a41-4106-b986-
> fc136d54c4c6%40googlegroups.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>

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

Re: Automatic prefetching in querysets

2017-08-16 Thread Gordon Wrigley

Regarding 2, it does work for reverse one-to-one relations.

On Wed, Aug 16, 2017 at 9:17 PM, Aymeric Augustin <
aymeric.augus...@polytechnique.org> wrote:

> On 15 Aug 2017, at 11:44, Gordon Wrigley  wrote:
>
> I'd like to discuss automatic prefetching in querysets. Specifically
> automatically doing prefetch_related where needed without the user having
> to request it.
>
>
>
> Hello,
>
> I'm rather sympathetic to this proposal. Figuring out N + 1 problems in
> the admin or elsewhere gets old.
>
>
> In addition to everything that was said already, I'd like to point out
> that Django already has a very similar "magic auto prefetching" behavior in
> some cases :-)
>
> I'm referring to the admin which calls select_related() on non-nullable
> foreign keys in the changelist view. The "non-nullable" condition makes
> that behavior hard to predict — I'd go as far as to call it non
> deterministic. For details, see slide 54 of https://myks.org/data/
> 20161103-Django_Under_the_Hood-Debugging_Performance.pdf and the audio
> commentary at https://youtu.be/5fheDDj3oHY?t=2024.
>
>
> The feature proposed here is most useful if it's opt-out because it
> targets people who aren't aware that the problem even exists — at best they
> notice that Django is slow and that reminds them vaguely of a rant that
> explains why ORMs are the worst thing since object oriented programming.
>
> It should kick in only when no select_related or prefetch_related is in
> effect, to avoid interfering with pre-existing optimizations. It's still
> easy to construct an example where it would degrade performance but I don't
> think such situations will be common in practice. Still, there should be a
> per-queryset opt-out for these cases.
>
> We may want to introduce it with a deprecation path, that is, make it
> opt-in at first and log a deprecation warning where the behavior would
> kick-in, so developers who want to disable it can add the per-queryset
> opt-out.
>
>
> At this point, my main concerns are:
>
> 1) The difficulty of identifying where the queryset originates, given that
> querysets are lazy. Passing objects around is common; sometimes it can be
> hard to figure out where an object comes from. It isn't visible in the
> stack trace. In my opinion this is the strongest argument against the
> feature.
>
> 2) The lack of this feature for reverse one-to-one relations; it's only
> implemented for foreign keys. It's hard to tell them apart in Python code.
> The subtle differences, like return None vs. raise ObjectDoesNotExist when
> there's no related object, degrade the developer experience.
>
> 3) The strong opinions expressed against the feature. I'm not sure that
> consensus is within reach. If we can't agree that this is an adequate
> amount of magic, we're likely to stick with the status quo. I'd rather not
> have this question decided by a vote of the technical board.
>
>
> In the grand scheme of things, going from "prefetching a related instance
> for an object" to "prefetching related instances for all objects in the
> queryset" isn't that much of a stretch... But I admit it's rather scary to
> make this change for all existing Django projects!
>
>
> Best regards,
>
> --
> Aymeric.
>
>
>
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit https://groups.google.com/d/
> topic/django-developers/EplZGj-ejvg/unsubscribe.
> To unsubscribe from this group and all its topics, 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 https://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/django-developers/2D9DCC0F-EAB0-4512-8AEC-
> 08A694DF9074%40polytechnique.org
> <https://groups.google.com/d/msgid/django-developers/2D9DCC0F-EAB0-4512-8AEC-08A694DF9074%40polytechnique.org?utm_medium=email&utm_source=footer>
> .
>
> For more options, visit https://groups.google.com/d/optout.
>

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

Re: Automatic prefetching in querysets

2017-08-16 Thread Gordon Wrigley

 going
>> to know about a global setting and can opt in to the old behaviour rather
>> easily. Newer users that do not know about select/prefetch_related or these
>> settings will fall into the new behaviour by default.
>>
>> It's unreasonable to expect every user of django learn the ins and outs
>> of all queryset methods. I'm probably considered a django orm expert, and I
>> still sometimes write queries that are non-optimal or *become* non-optimal
>> after changes in unrelated areas. At an absolute minimum we should be
>> screaming and shouting when this happens. But we can also fix the issue
>> while complaining, and help guide users into correct behaviour.
>>
>>
>> On Wednesday, 16 August 2017 08:41:31 UTC+10, Anthony King wrote:
>>>
>>> Automatically prefetching is something I feel should be avoided.
>>>
>>> A common gripe I have with ORMs is they hide what's actually happening
>>> with the database, resulting in beginners-going-on-intermediates
>>> building libraries/systems that don't scale well.
>>>
>>> We have several views in a dashboard, where a relation may be accessed
>>> once or twice while iterating over a large python filtered queryset.
>>> Prefetching this relation based on the original queryset has the
>>> potential to add around 5 seconds to the response time (probably more, that
>>> table has doubled in size since I last measured it).
>>>
>>> I feel it would be better to optimise for your usecase, as apposed to
>>> try to prevent uncalled-for behaviour.
>>>
>>>
>>>
>>> On Aug 15, 2017 23:15, "Luke Plant"  wrote:
>>>
>>>> I agree with Marc here that the proposed optimizations are 'magical'. I
>>>> think when it comes to optimizations like these you simply cannot know in
>>>> advance whether doing extra queries is going to a be an optimization or a
>>>> pessimization. If I can come up with a single example where it would
>>>> significantly decrease performance (either memory usage or speed) compared
>>>> to the default (and I'm sure I can), then I would be strongly opposed to it
>>>> ever being default behaviour.
>>>>
>>>> Concerning implementing it as an additional  QuerySet method like
>>>> `auto_prefetch()` - I'm not sure what I think, I feel like it could get
>>>> icky (i.e. increase our technical debt), due to the way it couples things
>>>> together. I can't imagine ever wanting to use it, though, I would always
>>>> prefer the manual option.
>>>>
>>>> Luke
>>>>
>>>>
>>>>
>>>> On 15/08/17 21:02, Marc Tamlyn wrote:
>>>>
>>>> Hi Gordon,
>>>>
>>>> Thanks for the suggestion.
>>>>
>>>> I'm not a fan of adding a layer that tries to be this clever. How would
>>>> possible prefetches be identified? What happens when an initial loop in a
>>>> view requires one prefetch, but a subsequent loop in a template requires
>>>> some other prefetch? What about nested loops resulting in nested
>>>> prefetches? Code like this is almost guaranteed to break unexpectedly in
>>>> multiple ways. Personally, I would argue that correctly setting up and
>>>> maintaining appropriate prefetches and selects is a necessary part of
>>>> working with an ORM.
>>>>
>>>> Do you know of any other ORMs which attempt similar magical
>>>> optimisations? How do they go about identifying the cases where it is
>>>> necessary?
>>>>
>>>> On 15 August 2017 at 10:44, Gordon Wrigley 
>>>> wrote:
>>>>
>>>>> I'd like to discuss automatic prefetching in querysets. Specifically
>>>>> automatically doing prefetch_related where needed without the user having
>>>>> to request it.
>>>>>
>>>>> For context consider these three snippets using the Question & Choice
>>>>> models from the tutorial
>>>>> <https://docs.djangoproject.com/en/1.11/intro/tutorial02/#creating-models>
>>>>>  when
>>>>> there are 100 questions each with 5 choices for a total of 500 choices.
>>>>>
>>>>> Default
>>>>> for choice in Choice.objects.all():
>>>>> print(choice.question.question_text, ':', choice.choice_text)
>>>>> 501 db queries, fetches 500

Re: Automatic prefetching in querysets

2017-08-15 Thread Gordon Wrigley

The warnings you propose would certainly be an improvement on the status
quo.
However for that to be a complete solution Django would also need to detect
places where there are redundant prefetch_relateds.

Additionally tools like the Admin and DRF would need to provide adequate
hooks for inserting these calls.
For example ModelAdmin.get_queryset is not really granular enough as it's
used by both the list and detail views which might touch quite different
sets of fields. (Although in practice what you generally do is optimize the
list view as that's the one that tends to explode)

That aside I sincerely believe that the proposed approach is superior to
the current default behavior in the majority of cases and further more
doesn't fail as badly as the current behavior when it's not appropriate. I
expect that if implemented as an option then in time that belief would
prove itself.

On Tue, Aug 15, 2017 at 8:17 PM, Tom Forbes  wrote:

> Exploding query counts are definitely a pain point in Django, anything to
> improve that is definitely worth considering. They have been a problem in
> all Django projects I have seen.
>
> However I think the correct solution is for developers to correctly add
> select/prefetch calls. There is no general solution for automatically
> applying them that works for enough cases, and i think adding such a method
> to querysets would be used incorrectly and too often.
>
> Perhaps a better solution would be for Django to detect these O(n) query
> cases and display intelligent warnings, with suggestions as to the correct
> select/prefetch calls to add. When debug mode is enabled we could detect
> repeated foreign key referencing from the same source.
>
> On 15 Aug 2017 19:44, "Gordon Wrigley"  wrote:
>
> Sorry maybe I wasn't clear enough about the proposed mechanism.
>
> Currently when you dereference a foreign key field on an object (so
> 'choice.question' in the examples above) if it doesn't have the value
> cached from an earlier access, prefetch_related or select_related then
> Django will automatically perform a db query to fetch it. After that the
> value will then be cached on the object for any future dereferences.
>
> This automatic fetching is the source the N+1 query problems and in my
> experience most gross performance problems in Django apps.
>
> The proposal essentially is to add a new queryset function that says for
> the group of objects fetched by this queryset, whenever one of these
> automatic foreign key queries happens on one of them instead of fetching
> the foreign key for just that one use the prefetch mechanism to fetch it
> for all of them.
> The presumption being that the vast majority of the time when you access a
> field on one object from a queryset result, probably you are going to
> access the same field on many of the others as well.
>
> The implementation I've used in production does nest across foreign keys
> so something (admittedly contrived) like:
> for choice in Choice.objects.all():
> print(choice.question.author)
> Will produce 3 queries, one for all choices, one for the questions of
> those choices and one for the authors of those questions.
>
> It's worth noting that because these are foreign keys in their "to one"
> direction each of those queryset results will be at most the same size (in
> rows) as the proceeding one and often (due to nulls and duplicates) smaller.
>
> I do not propose touching reverse foreign key or many2many fields as the
> generated queries could request substantially more rows from the DB than
> the original query and it's not at all clear how this mechanism would
> sanely interact with filtering etc. So this is purely about the forward
> direction of foreign keys.
>
> I hope that clarifies my thinking some.
>
> Regards
> G
>
> On Tue, Aug 15, 2017 at 7:02 PM, Marc Tamlyn 
> wrote:
>
>> Hi Gordon,
>>
>> Thanks for the suggestion.
>>
>> I'm not a fan of adding a layer that tries to be this clever. How would
>> possible prefetches be identified? What happens when an initial loop in a
>> view requires one prefetch, but a subsequent loop in a template requires
>> some other prefetch? What about nested loops resulting in nested
>> prefetches? Code like this is almost guaranteed to break unexpectedly in
>> multiple ways. Personally, I would argue that correctly setting up and
>> maintaining appropriate prefetches and selects is a necessary part of
>> working with an ORM.
>>
>> Do you know of any other ORMs which attempt similar magical
>> optimisations? How do they go about identifying the cases where it is
>> necessary?
>>
>> On 15 August 2017 at 10

Re: Automatic prefetching in querysets

2017-08-15 Thread Gordon Wrigley

I didn't answer your questions directly. Sorry for the quoting but it's the
easiest way to deal with a half dozen questions.

> How would possible prefetches be identified?

Wherever we currently automatically fetch a foreign key value.

> What happens when an initial loop in a view requires one prefetch, but a
subsequent loop in a template requires some other prefetch?

They each do whatever prefetch they need, just as a human optimizing this
would add two prefetch clauses.

> What about nested loops resulting in nested prefetches?

Nested loops only really come up when you are dealing with RelatedManagers
which are outside the scope of this. Or did you have some other nested loop
case in mind?

>  I would argue that correctly setting up and maintaining appropriate
prefetches and selects is a necessary part of working with an ORM.

Having been lead engineer on a code base of ~100,000 lines with over 100
calls to prefetch_related and a lot of tests specifically for finding
missing ones I'd argue it's one of the worst aspects of working with
Djangos ORM at non trivial scale.

> Do you know of any other ORMs which attempt similar magical optimisations?


I don't, but unlike Django where I have years of experience I have next to
no experience with other ORM's.

Regards G

On Tue, Aug 15, 2017 at 7:44 PM, Gordon Wrigley 
wrote:

> Sorry maybe I wasn't clear enough about the proposed mechanism.
>
> Currently when you dereference a foreign key field on an object (so
> 'choice.question' in the examples above) if it doesn't have the value
> cached from an earlier access, prefetch_related or select_related then
> Django will automatically perform a db query to fetch it. After that the
> value will then be cached on the object for any future dereferences.
>
> This automatic fetching is the source the N+1 query problems and in my
> experience most gross performance problems in Django apps.
>
> The proposal essentially is to add a new queryset function that says for
> the group of objects fetched by this queryset, whenever one of these
> automatic foreign key queries happens on one of them instead of fetching
> the foreign key for just that one use the prefetch mechanism to fetch it
> for all of them.
> The presumption being that the vast majority of the time when you access a
> field on one object from a queryset result, probably you are going to
> access the same field on many of the others as well.
>
> The implementation I've used in production does nest across foreign keys
> so something (admittedly contrived) like:
> for choice in Choice.objects.all():
> print(choice.question.author)
> Will produce 3 queries, one for all choices, one for the questions of
> those choices and one for the authors of those questions.
>
> It's worth noting that because these are foreign keys in their "to one"
> direction each of those queryset results will be at most the same size (in
> rows) as the proceeding one and often (due to nulls and duplicates) smaller.
>
> I do not propose touching reverse foreign key or many2many fields as the
> generated queries could request substantially more rows from the DB than
> the original query and it's not at all clear how this mechanism would
> sanely interact with filtering etc. So this is purely about the forward
> direction of foreign keys.
>
> I hope that clarifies my thinking some.
>
> Regards
> G
>
> On Tue, Aug 15, 2017 at 7:02 PM, Marc Tamlyn 
> wrote:
>
>> Hi Gordon,
>>
>> Thanks for the suggestion.
>>
>> I'm not a fan of adding a layer that tries to be this clever. How would
>> possible prefetches be identified? What happens when an initial loop in a
>> view requires one prefetch, but a subsequent loop in a template requires
>> some other prefetch? What about nested loops resulting in nested
>> prefetches? Code like this is almost guaranteed to break unexpectedly in
>> multiple ways. Personally, I would argue that correctly setting up and
>> maintaining appropriate prefetches and selects is a necessary part of
>> working with an ORM.
>>
>> Do you know of any other ORMs which attempt similar magical
>> optimisations? How do they go about identifying the cases where it is
>> necessary?
>>
>> On 15 August 2017 at 10:44, Gordon Wrigley 
>> wrote:
>>
>>> I'd like to discuss automatic prefetching in querysets. Specifically
>>> automatically doing prefetch_related where needed without the user having
>>> to request it.
>>>
>>> For context consider these three snippets using the Question & Choice
>>> models from the tutorial
>>> <https://docs.djangoproject.com/en/1.11/i

Re: Automatic prefetching in querysets

2017-08-15 Thread Gordon Wrigley

In my current version each object keeps a reference to a WeakSet of the
results of the queryset it came from.
This is populated in _fetch_all and if it is populated then
ForwardManyToOneDescriptor does a prefetch across all the objects in the
WeakSet instead of it's regular fetching.

On Tue, Aug 15, 2017 at 8:03 PM, Collin Anderson 
wrote:

> Hi Gordon,
>
> How is it implemented? Does each object keep a reference to the queryset
> it came from?
>
> Collin
>
> On Tue, Aug 15, 2017 at 2:44 PM, Gordon Wrigley 
> wrote:
>
>> Sorry maybe I wasn't clear enough about the proposed mechanism.
>>
>> Currently when you dereference a foreign key field on an object (so
>> 'choice.question' in the examples above) if it doesn't have the value
>> cached from an earlier access, prefetch_related or select_related then
>> Django will automatically perform a db query to fetch it. After that the
>> value will then be cached on the object for any future dereferences.
>>
>> This automatic fetching is the source the N+1 query problems and in my
>> experience most gross performance problems in Django apps.
>>
>> The proposal essentially is to add a new queryset function that says for
>> the group of objects fetched by this queryset, whenever one of these
>> automatic foreign key queries happens on one of them instead of fetching
>> the foreign key for just that one use the prefetch mechanism to fetch it
>> for all of them.
>> The presumption being that the vast majority of the time when you access
>> a field on one object from a queryset result, probably you are going to
>> access the same field on many of the others as well.
>>
>> The implementation I've used in production does nest across foreign keys
>> so something (admittedly contrived) like:
>> for choice in Choice.objects.all():
>> print(choice.question.author)
>> Will produce 3 queries, one for all choices, one for the questions of
>> those choices and one for the authors of those questions.
>>
>> It's worth noting that because these are foreign keys in their "to one"
>> direction each of those queryset results will be at most the same size (in
>> rows) as the proceeding one and often (due to nulls and duplicates) smaller.
>>
>> I do not propose touching reverse foreign key or many2many fields as the
>> generated queries could request substantially more rows from the DB than
>> the original query and it's not at all clear how this mechanism would
>> sanely interact with filtering etc. So this is purely about the forward
>> direction of foreign keys.
>>
>> I hope that clarifies my thinking some.
>>
>> Regards
>> G
>>
>> On Tue, Aug 15, 2017 at 7:02 PM, Marc Tamlyn 
>> wrote:
>>
>>> Hi Gordon,
>>>
>>> Thanks for the suggestion.
>>>
>>> I'm not a fan of adding a layer that tries to be this clever. How would
>>> possible prefetches be identified? What happens when an initial loop in a
>>> view requires one prefetch, but a subsequent loop in a template requires
>>> some other prefetch? What about nested loops resulting in nested
>>> prefetches? Code like this is almost guaranteed to break unexpectedly in
>>> multiple ways. Personally, I would argue that correctly setting up and
>>> maintaining appropriate prefetches and selects is a necessary part of
>>> working with an ORM.
>>>
>>> Do you know of any other ORMs which attempt similar magical
>>> optimisations? How do they go about identifying the cases where it is
>>> necessary?
>>>
>>> On 15 August 2017 at 10:44, Gordon Wrigley 
>>> wrote:
>>>
>>>> I'd like to discuss automatic prefetching in querysets. Specifically
>>>> automatically doing prefetch_related where needed without the user having
>>>> to request it.
>>>>
>>>> For context consider these three snippets using the Question & Choice
>>>> models from the tutorial
>>>> <https://docs.djangoproject.com/en/1.11/intro/tutorial02/#creating-models> 
>>>> when
>>>> there are 100 questions each with 5 choices for a total of 500 choices.
>>>>
>>>> Default
>>>> for choice in Choice.objects.all():
>>>> print(choice.question.question_text, ':', choice.choice_text)
>>>> 501 db queries, fetches 500 choice rows and 500 question rows from the
>>>> DB
>>>>
>>>> Prefetch_related
>>>> for choice in Choice.objects.

Re: Automatic prefetching in querysets

2017-08-15 Thread Gordon Wrigley

Sorry maybe I wasn't clear enough about the proposed mechanism.

Currently when you dereference a foreign key field on an object (so
'choice.question' in the examples above) if it doesn't have the value
cached from an earlier access, prefetch_related or select_related then
Django will automatically perform a db query to fetch it. After that the
value will then be cached on the object for any future dereferences.

This automatic fetching is the source the N+1 query problems and in my
experience most gross performance problems in Django apps.

The proposal essentially is to add a new queryset function that says for
the group of objects fetched by this queryset, whenever one of these
automatic foreign key queries happens on one of them instead of fetching
the foreign key for just that one use the prefetch mechanism to fetch it
for all of them.
The presumption being that the vast majority of the time when you access a
field on one object from a queryset result, probably you are going to
access the same field on many of the others as well.

The implementation I've used in production does nest across foreign keys so
something (admittedly contrived) like:
for choice in Choice.objects.all():
print(choice.question.author)
Will produce 3 queries, one for all choices, one for the questions of those
choices and one for the authors of those questions.

It's worth noting that because these are foreign keys in their "to one"
direction each of those queryset results will be at most the same size (in
rows) as the proceeding one and often (due to nulls and duplicates) smaller.

I do not propose touching reverse foreign key or many2many fields as the
generated queries could request substantially more rows from the DB than
the original query and it's not at all clear how this mechanism would
sanely interact with filtering etc. So this is purely about the forward
direction of foreign keys.

I hope that clarifies my thinking some.

Regards
G

On Tue, Aug 15, 2017 at 7:02 PM, Marc Tamlyn  wrote:

> Hi Gordon,
>
> Thanks for the suggestion.
>
> I'm not a fan of adding a layer that tries to be this clever. How would
> possible prefetches be identified? What happens when an initial loop in a
> view requires one prefetch, but a subsequent loop in a template requires
> some other prefetch? What about nested loops resulting in nested
> prefetches? Code like this is almost guaranteed to break unexpectedly in
> multiple ways. Personally, I would argue that correctly setting up and
> maintaining appropriate prefetches and selects is a necessary part of
> working with an ORM.
>
> Do you know of any other ORMs which attempt similar magical optimisations?
> How do they go about identifying the cases where it is necessary?
>
> On 15 August 2017 at 10:44, Gordon Wrigley 
> wrote:
>
>> I'd like to discuss automatic prefetching in querysets. Specifically
>> automatically doing prefetch_related where needed without the user having
>> to request it.
>>
>> For context consider these three snippets using the Question & Choice
>> models from the tutorial
>> <https://docs.djangoproject.com/en/1.11/intro/tutorial02/#creating-models> 
>> when
>> there are 100 questions each with 5 choices for a total of 500 choices.
>>
>> Default
>> for choice in Choice.objects.all():
>> print(choice.question.question_text, ':', choice.choice_text)
>> 501 db queries, fetches 500 choice rows and 500 question rows from the DB
>>
>> Prefetch_related
>> for choice in Choice.objects.prefetch_related('question'):
>> print(choice.question.question_text, ':', choice.choice_text)
>> 2 db queries, fetches 500 choice rows and 100 question rows from the DB
>>
>> Select_related
>> for choice in Choice.objects.select_related('question'):
>> print(choice.question.question_text, ':', choice.choice_text)
>> 1 db query, fetches 500 choice rows and 500 question rows from the DB
>>
>> I've included select_related for completeness, I'm not going to propose
>> changing anything about it's use. There are places where it is the best
>> choice and in those places it will still be up to the user to request it. I
>> will note that anywhere select_related is optimal prefetch_related is still
>> better than the default and leave it at that.
>>
>> The 'Default' example above is a classic example of the N+1 query
>> problem, a problem that is widespread in Django apps.
>> This pattern of queries is what new users produce because they don't know
>> enough about the database and / or ORM to do otherwise.
>> Experieced users will also often produce this because it's no

Automatic prefetching in querysets

2017-08-15 Thread Gordon Wrigley

I'd like to discuss automatic prefetching in querysets. Specifically
automatically doing prefetch_related where needed without the user having
to request it.

For context consider these three snippets using the Question & Choice
models from the tutorial
when
there are 100 questions each with 5 choices for a total of 500 choices.

Default
for choice in Choice.objects.all():
print(choice.question.question_text, ':', choice.choice_text)
501 db queries, fetches 500 choice rows and 500 question rows from the DB

Prefetch_related
for choice in Choice.objects.prefetch_related('question'):
print(choice.question.question_text, ':', choice.choice_text)
2 db queries, fetches 500 choice rows and 100 question rows from the DB

Select_related
for choice in Choice.objects.select_related('question'):
print(choice.question.question_text, ':', choice.choice_text)
1 db query, fetches 500 choice rows and 500 question rows from the DB

I've included select_related for completeness, I'm not going to propose
changing anything about it's use. There are places where it is the best
choice and in those places it will still be up to the user to request it. I
will note that anywhere select_related is optimal prefetch_related is still
better than the default and leave it at that.

The 'Default' example above is a classic example of the N+1 query problem,
a problem that is widespread in Django apps.
This pattern of queries is what new users produce because they don't know
enough about the database and / or ORM to do otherwise.
Experieced users will also often produce this because it's not always
obvious what fields will and won't be used and subsequently what should be
prefetched.
Additionally that list will change over time. A small change to a template
to display an extra field can result in a denial of service on your DB due
to a missing prefetch.
Identifying missing prefetches is fiddly, time consuming and error prone.
Tools like django-perf-rec
(which I was involved in creating) and nplusone
exist in part to flag missing
prefetches introduced by changed code.
Finally libraries like Django Rest Framework and the Admin will also
produce queries like this because it's very difficult for them to know what
needs prefetching without being explicitly told by an experienced user.

As hinted at the top I'd like to propose changing Django so the default
code behaves like the prefetch_related code.
Longer term I think this should be the default behaviour but obviously it
needs to be proved first so for now I'd suggest a new queryset function
that enables this behaviour.

I have a proof of concept of this mechanism that I've used successfully in
production. I'm not posting it yet because I'd like to focus on desired
behavior rather than implementation details. But in summary, what it does
is when accessing a missing field on a model, rather than fetching it just
for that instance, it runs a prefetch_related query to fetch it for all
peer instances that were fetched in the same queryset. So in the example
above it prefetches all Questions in one query.

This might seem like a risky thing to do but I'd argue that it really isn't.
The only time this isn't superior to the default case is when you are post
filtering the queryset results in Python.
Even in that case it's only inferior if you started with a large number of
results, filtered basically all of them and the code is structured so that
the filtered ones aren't garbage collected.
To cover this rare case the automatic prefetching can easily be disabled on
a per queryset or per object basis. Leaving us with a rare downside that
can easily be manually resolved in exchange for a significant general
improvement.

In practice this thing is almost magical to work with. Unless you already
have extensive and tightly maintained prefetches everywhere you get an
immediate boost to virtually everything that touches the database, often
knocking orders of magnitude off page load times.

If an agreement can be reached on pursuing this then I'm happy to put in
the work to productize the proof of concept.

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

Re: delegating our static file serving

2016-01-17 Thread Gordon

I put together a simple django middleware based off the functionality of 
whitenoise except that it uses the staticfiles storage for finding/serving 
files.  I am sure there are lots of improvements to be made on it but it 
demonstrates using the filestorage api to accommodate remote static files.  
It is roughly 100 lines.  
https://gist.github.com/thenewguy/a92e05a3568fb6f1cc7c

On Saturday, January 16, 2016 at 6:11:46 PM UTC-5, Gordon wrote:
>
> Apologies if this has already been mentioned but I read this thread in 
> full awhile ago and now everything is collapsed.
>
> If staticfiles serving is brought into django proper it would be nice if 
> the storage api was used so that staticfiles can be stored remotely.  I 
> just ran into this looking at whitenoise so I figured I would mention it 
> here to be taken into consideration when putting an implementation together.
>
> On Friday, January 8, 2016 at 12:11:12 PM UTC-5, David Evans wrote:
>>
>> I've just pushed a simple middleware wrapper that lets you install 
>> WhiteNoise by adding:
>>
>> MIDDLEWARE_CLASSES = (
>>...
>> 'whitenoise.middleware.StaticFilesMiddleware',
>>...
>> )
>>
>>
>> See: 
>> https://github.com/evansd/whitenoise/blob/master/whitenoise/middleware.py
>>
>> No messing with wsgi.py!
>>
>> This approach will be marginally less efficient than handling things at 
>> the WSGI layer, but the integration is much cleaner.
>>
>> On Tuesday, 29 December 2015 17:31:02 UTC, David Evans wrote:
>>>
>>> I'd be very happy to help with this. Two things to mention:
>>>
>>> 1. When I first wrote WhiteNoise the FileResponse class didn't exist and 
>>> so the only way I could access `wsgi.file_wrapper` was by wrapping the wsgi 
>>> application directly and bypassing Django. Now we have the FileResponse 
>>> class it would be possible to implement WhiteNoise as standard Django 
>>> middleware without having to edit wsgi.py. I think this would be a much 
>>> cleaner approach, and I don't think it will be too much work to implement. 
>>> I should have time to look at this next week.
>>>
>>> 2. Serving media files is a slightly different case. WhiteNoise was 
>>> designed around the assumption that it's serving a set of 
>>> developer-supplied, public files which remain unchanged during the lifetime 
>>> of the process. This simplifies a lot of performance and security concerns 
>>> that come with serving mutable, user-supplied files. At present, if you use 
>>> `add_files(settings.MEDIA_ROOT, prefix=settings.MEDIA_URL)` as you 
>>> suggested then WhiteNoise won't pick up any files that were uploaded after 
>>> the application was started -- at least, not unless you enable the 
>>> "autorefresh" setting which I explicitly don't recommend for production.
>>>
>>> Obviously it's possible to support this if we decide this is an 
>>> important goal but it will need more thought and will definitely be more 
>>> work than just supporting static files.
>>>
>>> On Tuesday, 29 December 2015 00:36:06 UTC, Tim Graham wrote:
>>>>
>>>> I'd like to work together with Dave to develop a proof of concept that 
>>>> integrates whitenoise into Django. I spent about an hour looking through 
>>>> whitenoise and our own static file serving code, and I think integrating 
>>>> whitenoise will yield a simpler user experience with about the same amount 
>>>> of code as have now.
>>>>
>>>> Essentially, we'd recommend adding something like this to existing 
>>>> wsgi.py files (it would be in the default startproject template)
>>>>
>>>> from whitenoise.django import DjangoWhiteNoise
>>>> application = DjangoWhiteNoise(application)
>>>> application.add_files(settings.MEDIA_ROOT, prefix=settings.MEDIA_URL)
>>>>
>>>> which would have the benefit of working out of the box in production 
>>>> too, I think. Of course, you could disable that based on settings.DEBUG or 
>>>> some other toggle.
>>>>
>>>> We could then deprecate:
>>>>
>>>> * django/contrib/staticfiles/views.py
>>>> * django/contrib/staticfiles/management/commands/runserver.py
>>>> * django/contrib/staticfiles/handlers.py
>>>> * django/views/static.py
>>>>
>>>> Any objections to doing further investigation in this area?
>>>>
>>>

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

Re: delegating our static file serving

2016-01-16 Thread Gordon

Apologies if this has already been mentioned but I read this thread in full 
awhile ago and now everything is collapsed.

If staticfiles serving is brought into django proper it would be nice if 
the storage api was used so that staticfiles can be stored remotely.  I 
just ran into this looking at whitenoise so I figured I would mention it 
here to be taken into consideration when putting an implementation together.

On Friday, January 8, 2016 at 12:11:12 PM UTC-5, David Evans wrote:
>
> I've just pushed a simple middleware wrapper that lets you install 
> WhiteNoise by adding:
>
> MIDDLEWARE_CLASSES = (
>...
> 'whitenoise.middleware.StaticFilesMiddleware',
>...
> )
>
>
> See: 
> https://github.com/evansd/whitenoise/blob/master/whitenoise/middleware.py
>
> No messing with wsgi.py!
>
> This approach will be marginally less efficient than handling things at 
> the WSGI layer, but the integration is much cleaner.
>
> On Tuesday, 29 December 2015 17:31:02 UTC, David Evans wrote:
>>
>> I'd be very happy to help with this. Two things to mention:
>>
>> 1. When I first wrote WhiteNoise the FileResponse class didn't exist and 
>> so the only way I could access `wsgi.file_wrapper` was by wrapping the wsgi 
>> application directly and bypassing Django. Now we have the FileResponse 
>> class it would be possible to implement WhiteNoise as standard Django 
>> middleware without having to edit wsgi.py. I think this would be a much 
>> cleaner approach, and I don't think it will be too much work to implement. 
>> I should have time to look at this next week.
>>
>> 2. Serving media files is a slightly different case. WhiteNoise was 
>> designed around the assumption that it's serving a set of 
>> developer-supplied, public files which remain unchanged during the lifetime 
>> of the process. This simplifies a lot of performance and security concerns 
>> that come with serving mutable, user-supplied files. At present, if you use 
>> `add_files(settings.MEDIA_ROOT, prefix=settings.MEDIA_URL)` as you 
>> suggested then WhiteNoise won't pick up any files that were uploaded after 
>> the application was started -- at least, not unless you enable the 
>> "autorefresh" setting which I explicitly don't recommend for production.
>>
>> Obviously it's possible to support this if we decide this is an important 
>> goal but it will need more thought and will definitely be more work than 
>> just supporting static files.
>>
>> On Tuesday, 29 December 2015 00:36:06 UTC, Tim Graham wrote:
>>>
>>> I'd like to work together with Dave to develop a proof of concept that 
>>> integrates whitenoise into Django. I spent about an hour looking through 
>>> whitenoise and our own static file serving code, and I think integrating 
>>> whitenoise will yield a simpler user experience with about the same amount 
>>> of code as have now.
>>>
>>> Essentially, we'd recommend adding something like this to existing 
>>> wsgi.py files (it would be in the default startproject template)
>>>
>>> from whitenoise.django import DjangoWhiteNoise
>>> application = DjangoWhiteNoise(application)
>>> application.add_files(settings.MEDIA_ROOT, prefix=settings.MEDIA_URL)
>>>
>>> which would have the benefit of working out of the box in production 
>>> too, I think. Of course, you could disable that based on settings.DEBUG or 
>>> some other toggle.
>>>
>>> We could then deprecate:
>>>
>>> * django/contrib/staticfiles/views.py
>>> * django/contrib/staticfiles/management/commands/runserver.py
>>> * django/contrib/staticfiles/handlers.py
>>> * django/views/static.py
>>>
>>> Any objections to doing further investigation in this area?
>>>
>>

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

Re: Why is from_db_value not called on ModelClass.objects.create()?

2015-09-28 Thread Gordon

Thanks for the replies and suggestions Simon and Marten.

I am definitely calling form.clean() because I am using the default admin
for views in my tests. The field in the case that was tripping me up is
being used as an auto-populating primary key... so the admin form was
excluding it from clean() and then redirecting to the wrong url.

Since the behavior is written and working as intended I can tackle this
particular problem from a different direction. But in my mind, it still
seems like ModelClass.objects.create() should return an instance as if it
were pulled from the database... especially with convenience methods like
`get_or_create` because the field value would be different in this scenario
depending on if the method uses get or create to return the instance (I
haven't looked at the implementation but the name would imply it relies on
the create method). But I guess there are cases where you don't need to
use the returned instance and would waste a few cpu cycles. This issue
makes `get_or_create` a little dangerous I think.

I think it adheres to the principle of least surprise since it makes Django
> models behave just like normal Python classes in this regard.
>

I am not sure about this since ModelClass.objects.create() is a manager
method that returns a new model instance and not the manager "self". But
after rereading the documentation about the `create` method it does provide
an alternate equivalent syntax that I would expect this behavior from.

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

Why is from_db_value not called on ModelClass.objects.create()?

2015-09-28 Thread Gordon

I originally asked this on IRC.  It was suggested to raise the question in 
the mailing list. I've copied the log below with irrelevant lines 
removed (dpastes provided after the quoted irc)

 Why is from_db_value not called on Cls.objects.create()?  For example, 
> in the CashModel test, why isn't it tested that cash is an instance of Cash 
> after create? 

 
> https://github.com/django/django/blob/master/tests/from_db_value/tests.py

 Something like http://dpaste.com/1R69S5F  I seem to be getting the 
> wrong value for a custom primary key in the admin because of this

 *only on the create redirect

 Or is this expected to be handled by the field?

 gp: this seems similar to #24028

 timograhm: would that test 'test_create()' in the dpaste fit in with 
> the from_db_value?  Or should the create test be somewhere else?

 seems okay there, does it pass or are you proposing a behavior 
> change?

 When I was trying to figure out why my values were incorrect I tried 
> that test and it failed.  But I would need to verify in a clean project 
> before sending a pr

 but I think the actual fix is outside of my knowledge

 This ugly hack seems to fix it on my field if that means anything to 
> you http://dpaste.com/3J0C5DD

 gp: it seems like it could be a useful behavior. I guess it 
> has probably been discussed before but I'm not aware of the discussion. 
> Maybe it would be worth raising on the mailing list and at least 
> documenting the reasons for the current behavior if it can't be changed due 
> to backwards compatibility.


 The first dpaste (1R69S5F) is a copy of the from_db_value/tests.py with 
the following modifications:

class FromDBValueTest(TestCase):
def setUp(self):
self.obj = CashModel.objects.create(cash='12.50')

def test_create(self):
self.assertIsInstance(self.obj.cash, Cash)


The second dpaste (3J0C5DD) is the following:

def contribute_to_class(self, cls, name):
super(IntegerIdentifierBase, self).contribute_to_class(cls, name)
cls_save = cls.save
def save_wrapper(obj, *args, **kwargs):
cls_save(obj, *args, **kwargs)
value = getattr(obj, self.attname)
value = self.to_python(value)
setattr(obj, self.attname, value)
cls.save = save_wrapper



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

Re: django admin: open popups as modals instead of windows

2015-02-25 Thread Gordon

I just wanted to add my 2cents worth as an opinion in favor of modals 
replacing popups for many of the admin links.

I am in no way claiming that a poorly implemented modal window is superior 
to a new window...  and there are certainly cases where a new window is 
preferable to the "perfect" modal.  Taking that into consideration, assume 
for the remainder of my reply that the modal popup is well implemented and 
addresses all raised usability concerns.

In my experience, modals can be very beneficial to users that are not 
technically inclined.  From a UX perspective, this is important to 
consider.  As long as the modal window is implemented using unobtrusive 
javascript, "power users" can be expected to know (or at least be 
taught/shown) that they can click with the middle mouse button to open a 
new tab or right click to select between new tab and new window instead of 
the default modal.  During my encounters with the fabled "technically 
challenged" user, I have observed that they tend to get confused (or at the 
very least have trouble navigating) when several windows are opened.  They 
are not proficient with alt-tab or any other window management techniques 
that you and I are comfortable with.  For this class of user, it makes 
sense to select the most appropriate default action for their click 
whenever possible.

I would counter the argument about native windows being superior with the 
following: the browser isn't aware of how tightly related the linked 
content is to the current page.  As such, it can't decide that the link 
should be opened as a modal inside of the current browsing context instead 
of a new window.  This means that it must fall back to the more generic of 
choices to cover all cases.  However, the content author does know the 
relation between the current page and the linked page.  Because of this 
knowledge, it makes sense that the author can make a better default 
selection than the browser when choosing a context in which to open a 
particular link.

For the sake of demonstration, I've found jQuery mobile to be an excellent 
choice when dealing with non-technical users.  Just to be clear, I am not 
advocating for JQM to be used in the admin with this reply.  With that 
being said, here is a link to examples of modal dialogs that I am fond of: 
http://demos.jquerymobile.com/1.4.5/pages-dialog/

On Wednesday, February 25, 2015 at 5:41:38 AM UTC-5, Russell Keith-Magee 
wrote:
>
>
> On Wed, Feb 25, 2015 at 10:39 AM, Loïc Bistuer  > wrote:
>
>> >
>> > On Feb 25, 2015, at 09:07, Russell Keith-Magee > > wrote:
>> >
>> > I have an operating system with a graphical user interface. The 
>> developers of that operating system spent an immense amount of time 
>> developing it, polishing it, making it behave in predictable ways, getting 
>> keyboard accessibility sorted out, and so on. The idea of a CSS+HTML+JS 
>> implementation of UI features that badly implement half of the behavior 
>> provided natively by the OS - and the idea that this implementation is 
>> somehow *preferable* to native UI elements - absolutely *boggles* my mind.
>>
>> While I agree on desktop OS, I find responsive HTML modals much more 
>> usable on mobile.
>>
>>  
> Weird - I've found the exact opposite, especially if you're talking about 
> a site that doesn't have a mobile-optimised website. I've given up counting 
> the number of websites that have popups that are larger than the screen 
> size of the mobile device... and then helpfully keep the modal window 
> centred on the screen so that the dismiss button is off the screen.
>
> That said, HTML modals vs popups is hardly the biggest issue we have when 
>> it comes to usability on mobile platforms, which isn't surprising 
>> considering the admin look & feel was invented way before mobile was a 
>> thing.
>>
>
> Very much agreed on this point.
>
> Russ %-) 
>

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

Re: Removing formtools from contrib (and call for maintainers)

2014-07-15 Thread Gordon

I mentioned this in the other thread but I will post here as well since it 
has moved.  I would be interested in being part of this.  Is there any sort 
of timeline on the move or is it just a proposal at this time?

On Saturday, July 12, 2014 12:55:52 PM UTC-4, Jannis Leidel wrote:
>
> Hi all, 
>
> I’m one of the core devs who spent a non-trivial amount of time working on 
> formtools — last to revamp the form wizard implementation there back in 
> 2012 (based ironically on Stephan Jaeckel’s django-formwizard app). And 
> I’ve put a non-trivial amount of time into spinning out another contrib app 
> — localflavor – with a net positive effect. It’s what made me re-evaluate 
> the idea of putting more contrib apps in own repos under the django Github 
> umbrella project. django-localflavor has not only received a README as Russ 
> put it jokingly, but a complete documentation that didn’t exist in that 
> form before and a extensive test suite. This in turn has been possible by 
> many new non-core contributors (38 in total), including some of them who 
> have have received push permissions as well (e.g. Erik Romijn who is now a 
> Django core dev as well). 
>
> If there is a chance that some feel like we’re losing an integral part of 
> Django by moving formtools into an own app, I’m not going to pry it from 
> their hands. But if there is more interest in maintaining it outside than 
> inside of Django, I’m happy to help splitting it off. 
>
> To me the whole idea of “blessed” apps is nothing more than handwaving 
> right now because there are lots of unanswered hard questions about the 
> blessing process. Such a project would require a community site built, 
> maintainer team and enough lieutenants to make sure it doesn’t diverge too 
> much from Django core development. Heck, it even may require answering the 
> question “What is a good Django app?”, something which is in the spirit of 
> innovation a slippery slop at best. So for now that imaginary “index” is 
> the PyPI, as for any other Python package, and the “blessing” happens by 
> the community via the common tools of public opinion like reviews, blog 
> posts, conference talks, screencasts, etc. 
>
> Whether djangopackages.com is capable of being such a platform, I don’t 
> know. As far as I know it’s mostly self-service for the app authors, which 
> is also the reason why it has some incomplete data about app features (the 
> “grids”). 
>
> Jannis 
>
>
> On 12.07.2014, at 01:25, Russell Keith-Magee  > wrote: 
>
> > 
> > On Fri, Jul 11, 2014 at 10:52 PM, Tim Graham  > wrote: 
> > continuing 
> https://groups.google.com/d/topic/django-developers/km2xIHM-gIA/discussion 
> under a better subject (was "FormWizard needs confirmation step logic. 
> ticket #21644")... 
> > 
> > I'd like propose removing formtools from contrib. 
> > 
> > Reason to move it out: 
> > - Allow more maintainers next to Django core devs 
> > - Release individually from Django (perhaps more often) on PyPI as 
> django-formtools 
> > 
> > Reasons something should be in contrib (from Marc Tamlyn): 
> > 
> > - The application is of vital importance to the vast majority of Django 
> sites, and needs to be done "correctly". Examples include auth and 
> staticfiles, sessions. 
> > - The application closely depends on internal, undocumented features of 
> Django or is strongly intertwined with the core features like the ORM. Such 
> applications are often very difficult to maintain their feature support 
> across multiple versions of Django. Examples include gis, postgres, 
> contenttypes. 
> > 
> > (Marc again): "To me, formtools meets neither of these requirements. It 
> is not a 90%+ use case application like the admin or auth, and I don't 
> believe there is much to it that is not workable outside of Django itself. 
> It is useful, and does not deserve to be abandoned (like comments). Under 
> github.com/django seems ideal to me." 
> > 
> > The definition that we used "back in the day" was "Contrib is a 
> collection of optional, defacto standard implementations of common 
> patterns." 
> > 
> >  * Optional - if we deleted the directory, Django would still work, and 
> you could build the deleted functionality yourself. 
> >  * Defacto Standard - This is "the way you should do it". No point in 
> having multiple auth or session frameworks for 99% of users. 
> >  * Common Patterns - Things that needs to be done, so lets give them an 
> obvious way to do it. 
> > 
> > By that definition, formtools falls is caught as a common pattern. 
> Wizards may not be needed on every website, but Wizard functionality is 
> definitely something I've had to build a bunch of times, and it's fiddly to 
> build it yourself. Having an "in the box" way to do wizards was helpful for 
> those cases. 
> > 
> > One of the major reasons to do this back in the day was because Python 
> packaging was such a mess, and the only way to make sure people had a good 
> out-of-the-box experience was to package all

Re: FormWizard needs confirmation step logic. ticket #21644

2014-07-03 Thread Gordon


>
> If we did split it out, would you be interested in being a maintainer?
>
>
I was against this at first but the more I've thought about it, I would be 
interested.  Has anyone else expressed any interest or will I be the only 
maintainer?

I have another project coming up that will most likely use formwizard. 
 What type of time frame would it take to get all of this set up if django 
decides to split formtools from contrib?

-- 
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/37a54345-dbf9-4c5e-9303-5df23c8aca6c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: FormWizard needs confirmation step logic. ticket #21644

2014-06-02 Thread Gordon

How would that work?  Being part of contrib is a big plus for the app in many 
ways.  I am not familiar with another app that was removed from contrib that 
would show an overview of the process and if it was successful.

-- 
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/a5e02ac8-576a-4ed5-9c28-a57644a6143b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: FormWizard needs confirmation step logic. ticket #21644

2014-05-26 Thread Gordon



On Monday, May 26, 2014 6:04:31 PM UTC-4, Tim Graham wrote:
>
> My observation is that not many core developers seem interested in 
> contrib.formtools these days. It was added by Adrian in 2006, but as far as 
> I can see from the commit history, it hasn't received much love lately. If 
> would be a good candidate for deprecation from Django itself, in my opinion.
>
> On Monday, May 26, 2014 3:32:41 PM UTC-4, Gordon wrote:
>>
>> Hello,
>>
>> I have posted an implementation/proof of concept to ticket 21644 (
>> https://code.djangoproject.com/ticket/21644).  The implementation is 
>> currently written as a mixin because I need to be able to use it now and I 
>> don't like maintaining a custom fork of django for my projects.  If changed 
>> to update the base classes directly, the diff would be minimal.
>>
>> I am happy to work on FormWizard improvements because they are 
>> particularly useful for some of my projects, but I haven't had much of a 
>> response.  Since this is a contrib app bundled with django, I am hesitant 
>> to spend too much time on it without some sort of positive and/or 
>> constructive feedback from someone who can accept the changes.
>>
>> On topic... the brief summary for #21644 is that this allows for 
>> "confirmation" steps in a formwizard that have access to the data submitted 
>> for previous steps.  This is required for something like a checkout 
>> confirmation or anything where the user needs to be presented with data 
>> entered previously for acceptance to be meaningful.  It was important to my 
>> use case that the wizard supports multiple confirmation steps.  I've used a 
>> base class that confirmation forms should inherit from to detect when 
>> confirmation logic should be used.  This allows for a backwards 
>> compatible opt-in approach.
>>
>> Thanks,
>> Gordon
>>
>


Fair enough and thanks for the reply.  That is too bad though.  I've seen a 
good number of 3rd party apps that would benefit from using the 
NamedUrlWizardView (
https://docs.djangoproject.com/en/dev/ref/contrib/formtools/form-wizard/#usage-of-namedurlwizardview
).

-- 
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/5476d56f-39e4-4078-beb5-f2801751728b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

FormWizard needs confirmation step logic. ticket #21644

2014-05-26 Thread Gordon

Hello,

I have posted an implementation/proof of concept to ticket 21644
(https://code.djangoproject.com/ticket/21644). The implementation is
currently written as a mixin because I need to be able to use it now and I
don't like maintaining a custom fork of django for my projects. If changed
to update the base classes directly, the diff would be minimal.

I am happy to work on FormWizard improvements because they are particularly
useful for some of my projects, but I haven't had much of a response.
Since this is a contrib app bundled with django, I am hesitant to spend
too much time on it without some sort of positive and/or constructive
feedback from someone who can accept the changes.

On topic... the brief summary for #21644 is that this allows for
"confirmation" steps in a formwizard that have access to the data submitted
for previous steps. This is required for something like a checkout
confirmation or anything where the user needs to be presented with data
entered previously for acceptance to be meaningful. It was important to my
use case that the wizard supports multiple confirmation steps. I've used a
base class that confirmation forms should inherit from to detect when
confirmation logic should be used. This allows for a backwards compatible
opt-in
approach.

Thanks,
Gordon

--
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/c50ed39a-70a6-436e-8afa-d2ddb1325908%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Is the solution in #12235 correct?

2013-02-22 Thread Gordon

After a chat on IRC I submitted ticket 
https://code.djangoproject.com/ticket/19888

On Friday, February 22, 2013 5:07:25 PM UTC-5, Gordon wrote:
>
> Hello,
>
> I don't think the issue is fixed.
>
> I am trying to use a UUID primary key.  Per 
> https://code.djangoproject.com/ticket/12235, I've marked my field as the 
> autofield in contribute_to_class.  However, now when I try to save a new 
> instance, django is trying to insert an integer into my uuid because it 
> thinks it is an autofield.  
>
> This causes a ValueError at /admin/…/…/add/ because django auto assigns an 
> integer for the field value.
>
> If forcing my uuid field to be an autofield is actually the correct 
> approach, how do I override what django inserts as the automatic value?
>

-- 
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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Is the solution in #12235 correct?

2013-02-22 Thread Gordon

 

Hello,

I don't think the issue is fixed.

I am trying to use a UUID primary key.  Per 
https://code.djangoproject.com/ticket/12235, I've marked my field as the 
autofield in contribute_to_class.  However, now when I try to save a new 
instance, django is trying to insert an integer into my uuid because it 
thinks it is an autofield.  

This causes a ValueError at /admin/…/…/add/ because django auto assigns an 
integer for the field value.

If forcing my uuid field to be an autofield is actually the correct 
approach, how do I override what django inserts as the automatic value?

-- 
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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

issue 15644

2012-03-12 Thread Gordon

Is this bug fix too late for the upcoming release?

https://code.djangoproject.com/ticket/15644

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

Re: Tests blocked by AssertionError 200 != 302 in login() calls

2009-12-05 Thread Gordon A

On Dec 5, 7:06 pm, Russell Keith-Magee  wrote:
> It's unclear if you're in the right place. Django-dev is for
> discussing the development of Django itself; django-users is for
> general user queries. Generally, if you're unsure, django-users is the
> right place to start.

Whoops.  Sorry.

> You haven't given specific details of exactly which tests are failing,
> but I suspect you're talking about the contrib.auth tests.

Yes.

> If password change tests are failing, it suggests that you have done
> something to change the way passwords are validated, and that change
> isn't compatible with Django. However, it's impossible to know what is
> going wrong without more details on exactly what you have done.

Yes, I'd written settings.py to take authentication from Apache2, and
your
answer is an important hint.  When I commented out my setting for
AUTHENTICATION_BACKEND setting, the tests ran fine.  I suppose that
should make me think about the state of authentication in my test
environment,
but I'll take any questions over to django-users.

Thanks very much for your help.

--

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

Tests blocked by AssertionError 200 != 302 in login() calls

2009-12-05 Thread Gordon A

Apologies if this isn't the right forum for this question.

When I run "manage.py test", I get many errors of this form:

FAIL: test_password_change_fails_with_invalid_old_password
  File 
  File tests/views.py", line 136, in login
AssertionError 200 != 302

The last line is always a call on a login() method, though the class
varies.  So the tests aren't getting past this one issue.

The error seems odd.  I would guess that response 200 would be good
news, but don't know Django's internals enough to say.  I would also
guess that the tests expect that the urls are set up so that these
calls are redirected before getting a response, and that the urls they
are actually meeting are handling the requests directly.  I don't know
what I would be doing in my settings.py file, or code, that would mess
with any of that.

What is expected of my code by the test so that it can get past this
login() call?

--

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

43 matches

Mail list logo