Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

Ok, I'm heading for work now, will give it a spin this evening.

Thanks everyone for your comments!

J

On Tuesday, 11 August 2015 02:07:07 UTC+10, Tim Graham wrote:
>
> Yes, I agree that a documentation change should be sufficient (although I 
> still didn't look at the formsets situation).
>
> On Monday, August 10, 2015 at 11:57:19 AM UTC-4, Carl Meyer wrote:
>>
>> Hi Tim, 
>>
>> On 08/10/2015 11:07 AM, Tim Graham wrote: 
>> > Yes, the idea is that ModelForm.save() method could become (after 
>> > deprecation finishes): 
>> > 
>> > def save(self): 
>> > """ 
>> > Save this form's self.instance object and M2M data. Return the 
>> model 
>> > instance. 
>> > """ 
>> > if self.errors: 
>> > raise ValueError( 
>> > "The %s could not be %s because the data didn't validate." 
>> % ( 
>> > self.instance._meta.object_name, 
>> > 'created' if self.instance._state.adding else 
>> 'changed', 
>> > ) 
>> > ) 
>> > self.instance.save() 
>> > self._save_m2m() 
>> > return self.instance 
>> > 
>> > Instead of calling super().save(commit=False) to retrieve 
>> form.instance, 
>> > you can just manipulate form.instance directly and then call 
>> > super().save(). I don't see a need for the _instance getters/setters 
>> you 
>> > proposed. 
>> > 
>> > There's also a deprecation checklist to help when writing a patch: 
>> > 
>> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature
>>  
>>
>> IMO this is an example of the sort of low-value deprecation of a 
>> widely-used API that gets people upset at us for wasting their time. 
>>
>> I agree that .save(commit=False) isn't the best or best-named API ever, 
>> but it's been in Django since the beginning, it's _extremely_ widely 
>> used, and (having taught Django classes to new users), I don't think it 
>> ranks very high on the list of Django APIs that cause confusion in 
>> practice. 
>>
>> I wouldn't mind an update to the docs to recommend manipulating 
>> `form.instance` and then calling `form.save()` (in place of `obj = 
>> form.save(commit=False); obj.save(); form.save_m2m()`). I don't think 
>> this requires any code changes; it's been true for a while (since 
>> model-validation I think?) that the actual instance-updating happens 
>> already in `_post_clean()`. 
>>
>> This could put us on the path toward an eventual deprecation of 
>> `commit=False`, but I think we should be very slow to actually deprecate 
>> it. 
>>
>> Carl 
>>
>>

-- 
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/934a01eb-03c1-40bf-a338-6e20c739bd5d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: CSRF_FAILURE_VIEW should not be used when DEBUG is False

[Luke Plant]

When users see a CSRF failure, it is almost always because of mistake 
made by the developer, and it is more useful under those circumstances 
for the users to see a more specific error message that will help 
developers rectify the problem. A generic 403 template is very unlikely 
to be helpful for this case. I haven't looked into it much, but probably 
it would be hard to pass the 'reason' to a generic template, without 
which developers will have a hard time debugging the problem.


Regards,

Luke

On 07/08/15 14:02, Tim Graham wrote:
I guess we might have to wait for Luke to reply to explain the 
reasoning for the original decision.


On Friday, August 7, 2015 at 8:47:23 AM UTC-4, Žan Anderle wrote:

That's true. But it still seems a bit off, that all other 4xx will
be handled diffrently with DEBUG=False. Shouldn't the default
behavior for csrf failure be the same? So while it's not a bug, I
still don't get it why I should update the view for CSRF failure,
while I only need to create 4xx.html template for other 4xx and
5xx statuses. Because the default CSRF failure template is not
very user friendly.

Dne petek, 07. avgust 2015 14.41.21 UTC+2 je oseba Tim Graham
napisala:

It doesn't seem to be a bug to me as there's nothing in the
documentation that says the view should only be used when
DEBUG=True. The default template also includes logic to vary
the content based on DEBUG.

On Saturday, August 1, 2015 at 11:28:57 AM UTC-4, Žan Anderle
wrote:

Hey everyone!

I noticed today that CSRF_FAILURE_VIEW is used even when
DEBUG=False. I'm not sure why this is so and I think it
would make much more sense to use default 403 handling
when DEBUG=False.

I checked the ticket where this was initially added, to
see if there was a particular reason for it.

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


I could only find one comment related to this. And
although it says 'Helpful 403 page if DEBUG is True'
that's not really the case.

I wanted to check here if there is a reason for this
behaviour before opening a ticket for it.

Have a great weekend :)
Žan

--
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/14115f91-7967-48b6-9843-586252fad624%40googlegroups.com 
.

For more options, visit https://groups.google.com/d/optout.


--
OSBORN'S LAW
Variables won't, constants aren't.

Luke Plant || http://lukeplant.me.uk/

--
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/55C8F37B.9010209%40cantab.net.
For more options, visit https://groups.google.com/d/optout.

Re: Future of the development server's auto-reloading

[George Kappel]

In case it is useful as a reference, runserver_plus from django-extensions 
uses Werkzeug/Watchdog for reloading

http://django-extensions.readthedocs.org/en/latest/runserver_plus.html


On Monday, August 10, 2015 at 12:15:46 PM UTC-5, Carl Meyer wrote:
>
> Hi Aymeric, 
>
> I actually spent the PyCon 2015 sprints working on a generic 
> watcher/reloader for Python servers, built on top of watchdog: 
> https://github.com/carljm/wsgiwatcher 
>
> It should still be considered alpha/prototype code, but I think the 
> approach is promising. It uses a two-process model, where the master 
> process just monitors for file changes, and spawns a subprocess with the 
> actual server. The API is just a `run()` function that takes a dotted 
> import path to a serve-forever callable. 
>
> This model (as opposed to the single-process threads-only model used by 
> runserver) fixes the "killed by settings.py SyntaxError" problem, 
> because a failure in the server-running subprocess never kills the 
> master monitor process; the monitor just waits for the next file change 
> and tries again. 
>
> The question of which paths to monitor is orthogonal. Wsgiwatcher 
> currently follows the same "monitor everything in sys.modules" pattern 
> as runserver, but it could easily be modified to take a fixed directory 
> to watch instead. In fact that would simplify it quite a bit - currently 
> it has to use a multiprocessing Queue and a separate thread in the child 
> process to poll sys.modules and communicate the file list back to the 
> monitor process, and that could be removed. 
>
> The main potential issue with Wsgiwatcher's multi-process model (which I 
> haven't explored much yet) is how it interacts with multi-process 
> servers like gunicorn. This wouldn't be an issue for using it (or 
> something like it) with runserver. 
>
> I don't know that I'll have time to get back to Wsgiwatcher soon, but if 
> you (or Sam) are interested in working on alternative reloaders for 
> runserver, you're more than welcome to use it, improve it, and/or rip 
> off some code from it. 
>
> Carl 
>
> On 08/08/2015 05:53 PM, Aymeric Augustin wrote: 
> > While writing some horrific code [1] related to the development server’s 
> > auto-reloading, I realized that its design is fundamentally flawed and I 
> > think that we should reconsider it. 
> > 
> > The current implementation walks through the list of all imported Python 
> > modules, attempts to figure out which file they come from and builds a 
> > list of these files. Then it watches all these files for changes by 
> > checking if their mtime changed every second or, since 1.7, with inotify 
> > on Linux. I tried to do it with kqueue on BSD (read: OS X) but I failed. 
> > This stuff is hard. 
> > 
> > In addition to the source of imported Python modules, the autoreloader 
> > watches .mo files in order to catch translation changes and source files 
> > for modules that failed to import — it extracts them from exception 
> > tracebacks. This shows the limits of basing the list on imported Python 
> > modules. Even with such hacks, the auto-reloader will never handle all 
> > cases. Two examples: 
> > 
> > - It doesn’t survive a syntax error in the settings module. I have 
> > reasons to believe that this would be extremely messy to fix. 
> > - If a module reads a configuration file on disk at startup and caches 
> > it, the auto-reloader doesn’t detect changes to that file. 
> > 
> > So we’re stuck with cases where the development server doesn’t reload. I 
> > can’t say they’re edge cases; who never made a typo in a settings file? 
> > People who run tutorials report that it’s a very common cause of 
> > frustration for beginners. 
> > 
> > It's also wasteful. There’s little need to watch every file of every 
> > dependency in the virtualenv for changes. 
> > 
> > I think it would be much better to remove the auto-reloading logic from 
> > Django and have an external process watch the current working directory 
> > for changes. (When virtualenv is considered appropriate for people with 
> > exactly 0 days of Django experience under their belt, I think we can 
> > require a dependency for the development server’s auto-reloading.) 
> > 
> > The behavior would be much easier to describe: if you change something 
> > in your source tree, it reloads automatically; if you change something 
> > somewhere else, you must reload manually. It would also be quite easy to 
> > customize with a standard include / exclude mechanism to select which 
> > files should be watched. 
> > 
> > We aren’t in the business of writing production web servers, neither are 
> > we in the business of writing a filesystem watcher :-) Watchman [2] 
> > appears to be a robust solution. However I would prefer a pure-Python 
> > implementation that could easily be installed in a virtualenv. Does 
> > someone have experience in this area? 
> > 
> > I’m not making a concrete proposal as I haven’t studied the details. It 
> > t

Re: Future of the development server's auto-reloading

[Carl Meyer]

Hi Aymeric,

I actually spent the PyCon 2015 sprints working on a generic
watcher/reloader for Python servers, built on top of watchdog:
https://github.com/carljm/wsgiwatcher

It should still be considered alpha/prototype code, but I think the
approach is promising. It uses a two-process model, where the master
process just monitors for file changes, and spawns a subprocess with the
actual server. The API is just a `run()` function that takes a dotted
import path to a serve-forever callable.

This model (as opposed to the single-process threads-only model used by
runserver) fixes the "killed by settings.py SyntaxError" problem,
because a failure in the server-running subprocess never kills the
master monitor process; the monitor just waits for the next file change
and tries again.

The question of which paths to monitor is orthogonal. Wsgiwatcher
currently follows the same "monitor everything in sys.modules" pattern
as runserver, but it could easily be modified to take a fixed directory
to watch instead. In fact that would simplify it quite a bit - currently
it has to use a multiprocessing Queue and a separate thread in the child
process to poll sys.modules and communicate the file list back to the
monitor process, and that could be removed.

The main potential issue with Wsgiwatcher's multi-process model (which I
haven't explored much yet) is how it interacts with multi-process
servers like gunicorn. This wouldn't be an issue for using it (or
something like it) with runserver.

I don't know that I'll have time to get back to Wsgiwatcher soon, but if
you (or Sam) are interested in working on alternative reloaders for
runserver, you're more than welcome to use it, improve it, and/or rip
off some code from it.

Carl

On 08/08/2015 05:53 PM, Aymeric Augustin wrote:
> While writing some horrific code [1] related to the development server’s
> auto-reloading, I realized that its design is fundamentally flawed and I
> think that we should reconsider it.
> 
> The current implementation walks through the list of all imported Python
> modules, attempts to figure out which file they come from and builds a
> list of these files. Then it watches all these files for changes by
> checking if their mtime changed every second or, since 1.7, with inotify
> on Linux. I tried to do it with kqueue on BSD (read: OS X) but I failed.
> This stuff is hard.
> 
> In addition to the source of imported Python modules, the autoreloader
> watches .mo files in order to catch translation changes and source files
> for modules that failed to import — it extracts them from exception
> tracebacks. This shows the limits of basing the list on imported Python
> modules. Even with such hacks, the auto-reloader will never handle all
> cases. Two examples:
> 
> - It doesn’t survive a syntax error in the settings module. I have
> reasons to believe that this would be extremely messy to fix.
> - If a module reads a configuration file on disk at startup and caches
> it, the auto-reloader doesn’t detect changes to that file.
> 
> So we’re stuck with cases where the development server doesn’t reload. I
> can’t say they’re edge cases; who never made a typo in a settings file?
> People who run tutorials report that it’s a very common cause of
> frustration for beginners.
> 
> It's also wasteful. There’s little need to watch every file of every
> dependency in the virtualenv for changes.
> 
> I think it would be much better to remove the auto-reloading logic from
> Django and have an external process watch the current working directory
> for changes. (When virtualenv is considered appropriate for people with
> exactly 0 days of Django experience under their belt, I think we can
> require a dependency for the development server’s auto-reloading.)
> 
> The behavior would be much easier to describe: if you change something
> in your source tree, it reloads automatically; if you change something
> somewhere else, you must reload manually. It would also be quite easy to
> customize with a standard include / exclude mechanism to select which
> files should be watched.
> 
> We aren’t in the business of writing production web servers, neither are
> we in the business of writing a filesystem watcher :-) Watchman [2]
> appears to be a robust solution. However I would prefer a pure-Python
> implementation that could easily be installed in a virtualenv. Does
> someone have experience in this area?
> 
> I’m not making a concrete proposal as I haven’t studied the details. It
> this point I’d just like to hear what you think of the general concept.
> 
> Thanks!
> 
> -- 
> Aymeric.
> 
> [1] https://github.com/django/django/pull/5106
> [2] https://github.com/facebook/watchman
> [3] 
> http://tutorial.djangogirls.org/en/django_installation/index.html#virtual-environment
> 
> -- 
> 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 

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Tim Graham]

Yes, I agree that a documentation change should be sufficient (although I 
still didn't look at the formsets situation).

On Monday, August 10, 2015 at 11:57:19 AM UTC-4, Carl Meyer wrote:
>
> Hi Tim, 
>
> On 08/10/2015 11:07 AM, Tim Graham wrote: 
> > Yes, the idea is that ModelForm.save() method could become (after 
> > deprecation finishes): 
> > 
> > def save(self): 
> > """ 
> > Save this form's self.instance object and M2M data. Return the model 
> > instance. 
> > """ 
> > if self.errors: 
> > raise ValueError( 
> > "The %s could not be %s because the data didn't validate." % 
> ( 
> > self.instance._meta.object_name, 
> > 'created' if self.instance._state.adding else 'changed', 
> > ) 
> > ) 
> > self.instance.save() 
> > self._save_m2m() 
> > return self.instance 
> > 
> > Instead of calling super().save(commit=False) to retrieve form.instance, 
> > you can just manipulate form.instance directly and then call 
> > super().save(). I don't see a need for the _instance getters/setters you 
> > proposed. 
> > 
> > There's also a deprecation checklist to help when writing a patch: 
> > 
> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature
>  
>
> IMO this is an example of the sort of low-value deprecation of a 
> widely-used API that gets people upset at us for wasting their time. 
>
> I agree that .save(commit=False) isn't the best or best-named API ever, 
> but it's been in Django since the beginning, it's _extremely_ widely 
> used, and (having taught Django classes to new users), I don't think it 
> ranks very high on the list of Django APIs that cause confusion in 
> practice. 
>
> I wouldn't mind an update to the docs to recommend manipulating 
> `form.instance` and then calling `form.save()` (in place of `obj = 
> form.save(commit=False); obj.save(); form.save_m2m()`). I don't think 
> this requires any code changes; it's been true for a while (since 
> model-validation I think?) that the actual instance-updating happens 
> already in `_post_clean()`. 
>
> This could put us on the path toward an eventual deprecation of 
> `commit=False`, but I think we should be very slow to actually deprecate 
> it. 
>
> Carl 
>
>

-- 
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/135f86e0-6777-490d-a337-19666c045f01%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Carl Meyer]

Hi Tim,

On 08/10/2015 11:07 AM, Tim Graham wrote:
> Yes, the idea is that ModelForm.save() method could become (after
> deprecation finishes):
> 
> def save(self):
> """
> Save this form's self.instance object and M2M data. Return the model
> instance.
> """
> if self.errors:
> raise ValueError(
> "The %s could not be %s because the data didn't validate." % (
> self.instance._meta.object_name,
> 'created' if self.instance._state.adding else 'changed',
> )
> )
> self.instance.save()
> self._save_m2m()
> return self.instance
> 
> Instead of calling super().save(commit=False) to retrieve form.instance,
> you can just manipulate form.instance directly and then call
> super().save(). I don't see a need for the _instance getters/setters you
> proposed.
> 
> There's also a deprecation checklist to help when writing a patch:
> https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature

IMO this is an example of the sort of low-value deprecation of a
widely-used API that gets people upset at us for wasting their time.

I agree that .save(commit=False) isn't the best or best-named API ever,
but it's been in Django since the beginning, it's _extremely_ widely
used, and (having taught Django classes to new users), I don't think it
ranks very high on the list of Django APIs that cause confusion in practice.

I wouldn't mind an update to the docs to recommend manipulating
`form.instance` and then calling `form.save()` (in place of `obj =
form.save(commit=False); obj.save(); form.save_m2m()`). I don't think
this requires any code changes; it's been true for a while (since
model-validation I think?) that the actual instance-updating happens
already in `_post_clean()`.

This could put us on the path toward an eventual deprecation of
`commit=False`, but I think we should be very slow to actually deprecate it.

Carl

-- 
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/55C8C9CF.8060004%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Tim Graham]

Yes, the idea is that ModelForm.save() method could become (after 
deprecation finishes):

def save(self):
"""
Save this form's self.instance object and M2M data. Return the model
instance.
"""
if self.errors:
raise ValueError(
"The %s could not be %s because the data didn't validate." % (
self.instance._meta.object_name,
'created' if self.instance._state.adding else 'changed',
)
)
self.instance.save()
self._save_m2m()
return self.instance

Instead of calling super().save(commit=False) to retrieve form.instance, 
you can just manipulate form.instance directly and then call 
super().save(). I don't see a need for the _instance getters/setters you 
proposed.

There's also a deprecation checklist to help when writing a patch:
https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature

On Monday, August 10, 2015 at 8:30:51 AM UTC-4, Javier Candeira wrote:
>
> On Monday, 10 August 2015 21:36:27 UTC+10, Tim Graham wrote:
>>
>> "I like `.apply()`, since it can either suggest that it's applying the 
>> form's data to the instance, or applies the validation rules to the form 
>> data."
>>
>> I don't think it does either of those things though? I don't find the 
>> name intuitive for what it does. I think the purpose of 
>> "save(commit=False)" is for the case when you need to modify a model 
>> instance *outside* of the form. If you can modify the instance inside the 
>> form, just do so in save().
>>
>
> So the only thing it does is throw an exception if there is a validation 
> error, and prepare m2m() so the m2m data will be saved later? 
>  
>
>> What happened to the idea of removing the need for the commit keyword arg 
>> that you presented earlier? As I said, I thought that was pretty good, 
>> except some investigation of whether or not it'll work well for formsets is 
>> needed.
>>
>
> Good catch. I got distracted rebasing from your patch, then I realised I 
> don't know how deprecation works inside the Django project, and somewhere 
> along the way I guess I suffered a loss of ambition.
>
> I'll give the patch another spin, try to understand what the commit=False 
> codepath does that can be done when instantiating the form so save() 
> doesn't need the commit keyword. 
>
> What's your opinion on turning `ModelForm.instance` into a property that 
> would get/set an _instance attribute, and perform the same side effects as 
> save(commit=False) perform now when returning the model instance? To me 
> this looks like it could work with `FormSet.instances` too, and it would 
> avoid having to find a name for the method, since `apply` and 
> `get_updated_instance` did not work. `.instance` just gets and sets 
> instance/instances, and does whatever preparation is needed in the 
> background.
>
> Also, is there anywhere I have missed in the documentation that explains 
> how deprecation works in the project? If we go for such a break with the 
> existing API, my patch would need to keep the current API working for a 
> while, and the documentation would have to document both APIs in parallel 
> for that time, then deprecate the `commit=False` path.
>
> Thanks,
>
> J
>
>
> On Sunday, August 9, 2015 at 7:18:57 PM UTC-4, Javier Candeira wrote:
>>>
>>> On Saturday, 8 August 2015 02:38:09 UTC+10, Patryk Zawadzki wrote:

 2015-08-07 16:51 GMT+02:00 Martin Owens : 
 > Could we use something simple like 'update()' and 'commit()' to which 
 save 
 > would call both? 

 Or `.apply()` and `.commit()` not to suggest that the form gets 
 updated in any way. 

>>>
>>> I like `.apply()`, since it can either suggest that it's applying the 
>>> form's data to the instance, or applies the validation rules to the form 
>>> data. 
>>>
>>> J
>>>
>>

-- 
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/1c1e50c3-96f4-4ef1-b5b1-a95e5d89be24%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

On Monday, 10 August 2015 22:30:51 UTC+10, Javier Candeira wrote:
>
> Also, is there anywhere I have missed in the documentation that explains 
> how deprecation works in the project? If we go for such a break with the 
> existing API, my patch would need to keep the current API working for a 
> while, and the documentation would have to document both APIs in parallel 
> for that time, then deprecate the `commit=False` path.
>

Answering myself: 
http://django.readthedocs.org/en/latest/internals/release-process.html#deprecation-policy

J

-- 
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/5339bb4c-ff84-4770-ae6f-6d4611bb0daa%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

On Monday, 10 August 2015 21:36:27 UTC+10, Tim Graham wrote:
>
> "I like `.apply()`, since it can either suggest that it's applying the 
> form's data to the instance, or applies the validation rules to the form 
> data."
>
> I don't think it does either of those things though? I don't find the name 
> intuitive for what it does. I think the purpose of "save(commit=False)" is 
> for the case when you need to modify a model instance *outside* of the 
> form. If you can modify the instance inside the form, just do so in save().
>

So the only thing it does is throw an exception if there is a validation 
error, and prepare m2m() so the m2m data will be saved later? 
 

> What happened to the idea of removing the need for the commit keyword arg 
> that you presented earlier? As I said, I thought that was pretty good, 
> except some investigation of whether or not it'll work well for formsets is 
> needed.
>

Good catch. I got distracted rebasing from your patch, then I realised I 
don't know how deprecation works inside the Django project, and somewhere 
along the way I guess I suffered a loss of ambition.

I'll give the patch another spin, try to understand what the commit=False 
codepath does that can be done when instantiating the form so save() 
doesn't need the commit keyword. 

What's your opinion on turning `ModelForm.instance` into a property that 
would get/set an _instance attribute, and perform the same side effects as 
save(commit=False) perform now when returning the model instance? To me 
this looks like it could work with `FormSet.instances` too, and it would 
avoid having to find a name for the method, since `apply` and 
`get_updated_instance` did not work. `.instance` just gets and sets 
instance/instances, and does whatever preparation is needed in the 
background.

Also, is there anywhere I have missed in the documentation that explains 
how deprecation works in the project? If we go for such a break with the 
existing API, my patch would need to keep the current API working for a 
while, and the documentation would have to document both APIs in parallel 
for that time, then deprecate the `commit=False` path.

Thanks,

J


On Sunday, August 9, 2015 at 7:18:57 PM UTC-4, Javier Candeira wrote:
>>
>> On Saturday, 8 August 2015 02:38:09 UTC+10, Patryk Zawadzki wrote:
>>>
>>> 2015-08-07 16:51 GMT+02:00 Martin Owens : 
>>> > Could we use something simple like 'update()' and 'commit()' to which 
>>> save 
>>> > would call both? 
>>>
>>> Or `.apply()` and `.commit()` not to suggest that the form gets 
>>> updated in any way. 
>>>
>>
>> I like `.apply()`, since it can either suggest that it's applying the 
>> form's data to the instance, or applies the validation rules to the form 
>> data. 
>>
>> J
>>
>

-- 
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/aaa7f1c4-b82a-4517-986f-da02a2d2f605%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Future of the development server's auto-reloading

[Aymeric Augustin]

Hello Sam,

On 10 août 2015, at 11:34, Sam Cooke  wrote:

> I've thrown together a rough proof of concept[2] for runserver that uses 
> watchdog to watch for changes/additions/deletions to python files recursively 
> in the same directory as manage.py and restarts the server accordingly. It 
> could easily be extended to handle translations.

Interesting proof of concept!

I had in mind a more invasive approach. To remove almost all supporting code 
from Django, I’d like the implementation to look like:

(somewhere in the runserver command, with an hypothetical watchrun utility)

if autoreload:
cmd = sys.argv + ["--noreload"]
os.execlp("watchrun", "--include=*.py,*.mo", "--", *cmd)

Of course the actual implementation would have to be a bit smarter with the 
include / exclude patterns in order to allow customization.

This would make the reloading tool required if you want runserver to autoreload.

-- 
Aymeric.

-- 
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/B6C92264-4D7E-411A-BD15-0F03D9C31F89%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Tim Graham]

"I like `.apply()`, since it can either suggest that it's applying the 
form's data to the instance, or applies the validation rules to the form 
data."

I don't think it does either of those things though? I don't find the name 
intuitive for what it does. I think the purpose of "save(commit=False)" is 
for the case when you need to modify a model instance *outside* of the 
form. If you can modify the instance inside the form, just do so in save().

What happened to the idea of removing the need for the commit keyword arg 
that you presented earlier? As I said, I thought that was pretty good, 
except some investigation of whether or not it'll work well for formsets is 
needed.

On Sunday, August 9, 2015 at 7:18:57 PM UTC-4, Javier Candeira wrote:
>
> On Saturday, 8 August 2015 02:38:09 UTC+10, Patryk Zawadzki wrote:
>>
>> 2015-08-07 16:51 GMT+02:00 Martin Owens : 
>> > Could we use something simple like 'update()' and 'commit()' to which 
>> save 
>> > would call both? 
>>
>> Or `.apply()` and `.commit()` not to suggest that the form gets 
>> updated in any way. 
>>
>
> I like `.apply()`, since it can either suggest that it's applying the 
> form's data to the instance, or applies the validation rules to the form 
> data. 
>
> J
>

-- 
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/6b91059b-15fe-4f15-80aa-cea4d8952d5a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: #25227 Add utility method `get_updated_model()` to `ModelForm`

[Javier Candeira]

Ok, so I rebased on Tim Graham's refactor and bugfix of ModelForm, and now 
the API is as follows:

`.save()` commits to the database. In `ModelForm`, it returns the 
associated instance, and in `FormSet`, the list of associated instances. It 
runs `.apply()` first.

`.apply()` just returns the instance or the associated instances. It's also 
the bit that throws exceptions when data doesn't pass validation, and the 
part that prepares the m2m() method when the saving is not going to happen 
right now.

`.save(commit=False)` is the same as `.apply()`, they're synonyms.

`.save(commit=True)` is the same as `.save()`. 

I have also changed the title description of the Ticket, because I realised 
that the goal was to avoid the `save(commit=False)`, not to add the new 
method with a particular name. The new method was just a way to get at the 
goal.

Now in a view for a ThingForm you can do:

if form.is_valid():
thing = form.apply()
do_stuff_to(thing)
form.save()

or 

if form.is_valid():
thing = form.apply()
do_stuff_to(thing)
thing.save()
form.save_m2m()

or even this one, though it goes against tradition by ignoring the returned 
instance from form.save():

if form.is_valid():
form.apply()
do_stuff_to(form.instance)
form.save()   

And, in a subclass of the Thing model:

class Gizmo(Thing):
def save(self, commit=True):
gizmo = self.apply()
do_stuff_to(gizmo)
if commit:
return super(Gizmo, self).save()
else:
return gizmo

or you can override the apply() method, which to me looks cleaner, but if 
we were to go this way we'd require further intervention in the docs:

class Gizmo(Thing):
def apply(self):
gizmo = super(Gizmo, self).apply()
do_stuff_to(gizmo)
return gizmo

Depending on what the stuff done to gizmo is, whether it's instance 
manipulation prior to giving it to the developer in the view, or prior to 
committing it to the database after the user has manipulated it.

Cheers,

Javier

-- 
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/6265cb63-ab7c-489c-8776-d0cb90aba467%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Future of the development server's auto-reloading

[Sam Cooke]

Hi all,

I've had experience playing around with watchdog[1] before - I've only ever
used it on OS X but it should work cross platform. I've thrown together a
rough proof of concept[2] for runserver that uses watchdog to watch for
changes/additions/deletions to python files recursively in the same
directory as manage.py and restarts the server accordingly. It could easily
be extended to handle translations.

To avoid changing a lot of code I've just set it up to use watchdog if
available and fall back to the current method silently if not.

I'd be happy to turn this into a more complete solution if people think
it's the right route to go. We can choose to:
 - make watchdog required - it's a simple pip install
 - make watchdog required if you want runserver to autoreload
 - fall back to the current implementation if watchdog is not on the python
path (and potentially tell the user about watchdog)

Do you think it's worth carrying on down this path?

Sam

[1] https://pythonhosted.org/watchdog/
[2] https://github.com/django/django/compare/master...sdcooke:watchdog
(ignore the code quality - it was thrown together quickly!)

On Sun, 9 Aug 2015 at 22:22 Daniel Moisset  wrote:

> The idea sounds awesome. A couple of corner cases that should be taken in
> consideration:
>
>  - I think it is common for small simple projects (and people doing the
> tutorial) to have the sqlite db file in the same project directory. If
> we're watching for writes in every file in the project directory, every
> save() could trigger a reload
>  - It is also usual to have the media/ directory based on filesystem
> storage inside the project dir. So any project with file uploads could
> trigger a reload on each file upload.
>
> Possibly some exclusion rules with sane defaults could help here
>
> Regards,
>
> D.
>
>
>
> On Sat, Aug 8, 2015 at 6:53 PM, Aymeric Augustin <
> aymeric.augus...@polytechnique.org> wrote:
>
>> Hello,
>>
>> While writing some horrific code [1] related to the development server’s
>> auto-reloading, I realized that its design is fundamentally flawed and I
>> think that we should reconsider it.
>>
>> The current implementation walks through the list of all imported Python
>> modules, attempts to figure out which file they come from and builds a list
>> of these files. Then it watches all these files for changes by checking if
>> their mtime changed every second or, since 1.7, with inotify on Linux. I
>> tried to do it with kqueue on BSD (read: OS X) but I failed. This stuff is
>> hard.
>>
>> In addition to the source of imported Python modules, the autoreloader
>> watches .mo files in order to catch translation changes and source files
>> for modules that failed to import — it extracts them from exception
>> tracebacks. This shows the limits of basing the list on imported Python
>> modules. Even with such hacks, the auto-reloader will never handle all
>> cases. Two examples:
>>
>> - It doesn’t survive a syntax error in the settings module. I have
>> reasons to believe that this would be extremely messy to fix.
>> - If a module reads a configuration file on disk at startup and caches
>> it, the auto-reloader doesn’t detect changes to that file.
>>
>> So we’re stuck with cases where the development server doesn’t reload. I
>> can’t say they’re edge cases; who never made a typo in a settings file?
>> People who run tutorials report that it’s a very common cause of
>> frustration for beginners.
>>
>> It's also wasteful. There’s little need to watch every file of every
>> dependency in the virtualenv for changes.
>>
>> I think it would be much better to remove the auto-reloading logic from
>> Django and have an external process watch the current working directory for
>> changes. (When virtualenv is considered appropriate for people with exactly
>> 0 days of Django experience under their belt, I think we can require a
>> dependency for the development server’s auto-reloading.)
>>
>> The behavior would be much easier to describe: if you change something in
>> your source tree, it reloads automatically; if you change something
>> somewhere else, you must reload manually. It would also be quite easy to
>> customize with a standard include / exclude mechanism to select which files
>> should be watched.
>>
>> We aren’t in the business of writing production web servers, neither are
>> we in the business of writing a filesystem watcher :-) Watchman [2] appears
>> to be a robust solution. However I would prefer a pure-Python
>> implementation that could easily be installed in a virtualenv. Does someone
>> have experience in this area?
>>
>> I’m not making a concrete proposal as I haven’t studied the details. It
>> this point I’d just like to hear what you think of the general concept.
>>
>> Thanks!
>>
>> --
>> Aymeric.
>>
>> [1] https://github.com/django/django/pull/5106
>> [2] https://github.com/facebook/watchman
>> [3]
>> http://tutorial.djangogirls.org/en/django_installation/index.html#virtual-environment
>>
>> --
>> You recei

Re: Allow custom "BoundFields" on forms and make BoundField public API

['Moritz Sichert' via Django developers (Contributions to Django itself)]

I split the commit in my branch to make the actual changes more visible
and moved the example out of the branch into the gist.

I will look into the ticket and work out some documentation.

Am 08.08.2015 um 13:42 schrieb Tim Graham:
> Here is a ticket for making BoundField a public API:
> https://code.djangoproject.com/ticket/12856
> Writing that documentation seems like a good place to start.
> 
> I would suggest not to move around so much code in your branch as that
> makes it difficult to tell what (if anything) has changed.
> 
> On Saturday, August 8, 2015 at 7:34:52 AM UTC-4, Moritz S. wrote:
> 
> Hello,
> 
> I'd like to propose a new feature for forms.
> 
> Currently if you need a form field with some extra features you can
> just
> subclass from django.forms.Field and add all the features you need.
> 
> However as soon as you're in a template and want to access the field
> you
> will get a BoundField instead of your custom field. The need to
> differentiate BoundField and Field is clear but it would be nice if it
> was possible to also customize BoundField.
> 
> For this reason I propose adding a new method "bind_to_form()" to
> django.forms.Field. It takes a form and a name and returns a BoundField
> specific to the Field instance. The default implementation just returns
> an instance of the existing BoundField but now it is possible to
> customize it.
> 
> To make it easier, BoundField should become public API. That means it
> should be able to import it with "from django.forms import BoundField"
> in order to subclass it.
> 
> In [1] you can see an implementation of bind_to_form().
> I also wrote BoundChoiceField as an example that allows better
> access to
> the choices. In [2] you can see how this could be used.
> 
> Considering BoundField wasn't really public before, this change
> shouldn't bring any backwards incompatibilities since the default
> behaviour stays the same. In my test branch all tests pass.
> 
> I'd really like to hear your feedback on this.
> If this sounds reasonable I will start writing documentation and
> possibly drafting a DEP if that's needed.
> 
> Moritz
> 
> [1]: https://github.com/MoritzS/django/tree/bound-fields
> 
> [2]: https://gist.github.com/MoritzS/7bd792f2eaf37da28dfb
> 
> 

-- 
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/55C86E72.2050801%40googlemail.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature