Re: DJANGO_SETTINGS_FILE

2017-04-07 Thread James Bennett 
On Thu, Apr 6, 2017 at 5:22 PM, James Pic  wrote:

> Do I need this to deploy my projects ? No of course, because I use the
> prettiest way hhihihi ;) I'm more than happy to win a debate
>

Please don't do this. This does not make you look like someone who I could
constructively engage with to try to figure out what problem is being
reported and what a good solution would be.


> And this is
> how the drama begins, I can imagine them in front of Tobias.
>

Please also don't do this. You are perfectly well aware that people will
give one type of explanation to a new user they're helping, and quite
another to a mailing list of experienced developers discussing the merits
of different approaches. And regardless of the context-insensitivity of
what you're doing here, putting words in someone's mouth to try to make
them look bad just makes you look bad.

Meanwhile, Django has a way to avoid all the turmoil you've proposed in
your reply -- run 'startproject', get package with settings file inside,
run 'manage.py '. Several tools for deploying Django into
production even make use of this, providing custom management commands
which can be invoked to run/deploy/etc., and doing so requires no deep
dives into Python imports, Python packages, or environment variables. Of
course, a third-party package which develops its own independent
configuration mechanism will need to teach all of its users how to make use
of and deploy that configuration mechanism, but that's the responsibility a
third-party project takes on when it invents its own method.

-- 
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/CAL13Cg_JBcc6mYhH7un3LKgGFR22pZz%3D_kQyc0sB5dh4uq8g-g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Database "execute hooks" for instrumentation

2017-04-07 Thread Shai Berger 
On Friday 07 April 2017 17:47:51 Carl Meyer wrote:
> Hi Shai,
> 
> On 04/07/2017 06:02 AM, Shai Berger wrote:
> > This is an idea that came up during the djangocon-europe conference: Add
> > the ability to install general instrumentation hooks around the database
> > "execute" and "executemany" calls.
> > 
> > Such hooks would allow all sorts of interesting features. For one, they
> > could replace the current special-case allowing assertNumQueries &
> > friends to record queries out of debug mode (it's an ugly hack, really),
> > but they could also support my imagined use case -- a context-manager
> > which could prevent database access during execution of some code (I'm
> > thinking mostly of using it around "render()" calls and serialization,
> > to make sure all database access is being done in the view).
> 
> Another use-case is for preventing database access during tests unless
> specifically requested by the test (e.g. pytest-django does this,
> currently via monkeypatching).
> 

Yep. This feels right.

> > My idea for implementation is to keep a thread-local stack of context-
> > managers, and have them wrap each call of "execute". We could actually
> > even use one such context-manager instead of the existing
> > CursorDebugWrapper.
> 
> Why a new thread-local instead of explicitly per-connection and stored
> on the connection?
> 

Sorry for implying that it would be a new thread-local, I just hadn't thought 
it through when I wrote this. Of course it goes on the (already thread-local) 
connection.

Shai.

Re: Proposal: make Model __unicode__() default to self.name

2017-04-07 Thread Collin Anderson 
Hi All,

Looking at the code snippet, cls is a _local_ variable, not an instance
attribute (self.cls). If we changed it to an instance attribute, it would
lose most of the speed optimization. Also, self.cls would possibly clash
with a field named "cls".

Collin


On Fri, Apr 7, 2017 at 10:33 AM, Kapil Garg  wrote:

> The opened ticket is about Model.__str__ method. Should i open a new
> ticket for this change ?
> As i see in code, self.__class__ is used in a lot of places but will it
> effect optimization if we change lookups from self.__class__ to self.cls
>
> Because the methods where class is being used frequently, already store it
> in local variable and then make references to local variable.
>
> So should it really be changed ?
>
> On Fri, Apr 7, 2017, 6:52 PM Marco Silva  wrote:
>
>> I noticed this on the init
>>
>> def __init__(self, *args, **kwargs):
>>  # Alias some things as locals to avoid repeat global lookups
>>  cls = self.__class__
>>
>> maybe you should change it to self.cls??
>> Try to submit a PR to the open ticket.
>>
>> segunda-feira, 3 de Abril de 2017 às 21:07:47 UTC+1, Kapil Garg escreveu:
>>
>> So does this patch seem fine ?
>>
>> diff --git a/django/db/models/base.py b/django/db/models/base.py
>> index 3c1cb9a..f58e12b 100644
>> --- a/django/db/models/base.py
>> +++ b/django/db/models/base.py
>> @@ -504,7 +504,7 @@ class Model(metaclass=ModelBase):
>>  return '<%s: %s>' % (self.__class__.__name__, u)
>>
>>  def __str__(self):
>> -return '%s object' % self.__class__.__name__
>> +return '%s object pk=%s' % (self.__class__.__name__,
>> self._get_pk_val())
>>
>>  def __eq__(self, other):
>>  if not isinstance(other, Model):
>>
>>
>>
>>
>> On Monday, 3 April 2017 23:07:56 UTC+5:30, Collin Anderson wrote:
>>
>> I'd recommend not saying "unsaved". "new" if anything. UUID pk's may
>> default to generating a pk before save, so it might just be best to show
>> the actual current pk value
>>
>> On Mon, Apr 3, 2017 at 1:06 PM, Kapil Garg  wrote:
>>
>> So what should the final __str__ show: Should it be 'ClassName object
>> pk=Something' and if pk is None then should it be 'ClassName object
>> (unsaved)' or 'ClassName object pk=None' ?
>>
>> On Sunday, 2 April 2017 23:47:01 UTC+5:30, Collin Anderson wrote:
>>
>> Makes sense to me. Maybe still keep the "Transaction object" part, and
>> use None if no pk.
>>
>> On Sun, Apr 2, 2017 at 11:09 AM, Kapil Garg  wrote:
>>
>> Ticket 27953  is regarding
>> this proposal and the suggestion is about adding "pk" in Model string
>> representation if it exists.
>>
>> On Thursday, 11 July 2013 09:16:25 UTC+5:30, Collin Anderson wrote:
>>
>> Hi All,
>>
>> Have you ever quickly set up a model, ran syncdb, and added a few sample
>> objects in the admin to only see a bunch of "MyModel object"s in the
>> changelist? I always forget to add a __unicode__()/__str__() method on my
>> models.
>>
>> I ran "git grep -1 __unicode__" on some of my django projects and noticed
>> a lot of repeated code. In fact, it seems that in about a _third_ of all my
>> cases, I'm just returning self.name, or returning self.name would have
>> been a good default. I looked at a few 3rd party apps for comparison and
>> found similar results, though not for every app.
>>
>> IMHO, returning self.name (if the field or property exists) is a
>> sensible default for __unicode__. We can still return "MyModel object" if
>> there's no "name" attribute. You'll still end up adding your own
>> __unicode__ method much of the time, just like you always have.
>>
>> Yes, it's "magic", but we can document it.
>> Yes, it's a little more confusing, but we don't have to explain it during
>> the tutorial.
>> Yes, it's backwards incompatible, but only in rare cases should it be a
>> problem.
>> Yes, it could look like any Model without a "name" field is "wrong", but
>> it's not.
>> Yes, "title" is also very popular, but name is better. :)
>>
>> It has the effect of being a little more friendly in many cases, and can
>> result in more DRY code.
>>
>> What do your __unicode__/__str__ methods look like? Is this a bad idea?
>>
>> Thanks,
>> Collin
>>
>> --
>> 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-develop...@googlegroups.com.
>> To post to this group, send email to django-d...@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/c7254a96-7ee3-4262-a90b-
>> 83dcfe8fa3d4%40googlegroups.com
>>

Re: Database "execute hooks" for instrumentation

2017-04-07 Thread Carl Meyer 
Hi Shai,

On 04/07/2017 06:02 AM, Shai Berger wrote:
> This is an idea that came up during the djangocon-europe conference: Add the 
> ability to install general instrumentation hooks around the database 
> "execute" 
> and "executemany" calls.
> 
> Such hooks would allow all sorts of interesting features. For one, they could 
> replace the current special-case allowing assertNumQueries & friends to 
> record 
> queries out of debug mode (it's an ugly hack, really), but they could also 
> support my imagined use case -- a context-manager which could prevent 
> database 
> access during execution of some code (I'm thinking mostly of using it around 
> "render()" calls and serialization, to make sure all database access is being 
> done in the view).

Another use-case is for preventing database access during tests unless
specifically requested by the test (e.g. pytest-django does this,
currently via monkeypatching).

> My idea for implementation is to keep a thread-local stack of context-
> managers, and have them wrap each call of "execute". We could actually even 
> use one such context-manager instead of the existing CursorDebugWrapper.

Why a new thread-local instead of explicitly per-connection and stored
on the connection?

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 https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5d5ee8f1-6b22-c48c-7a6b-11eac2928b64%40oddbird.net.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: OpenPGP digital signature

Re: Proposal: make Model __unicode__() default to self.name

2017-04-07 Thread Kapil Garg 
The opened ticket is about Model.__str__ method. Should i open a new ticket
for this change ?
As i see in code, self.__class__ is used in a lot of places but will it
effect optimization if we change lookups from self.__class__ to self.cls

Because the methods where class is being used frequently, already store it
in local variable and then make references to local variable.

So should it really be changed ?

On Fri, Apr 7, 2017, 6:52 PM Marco Silva  wrote:

> I noticed this on the init
>
> def __init__(self, *args, **kwargs):
>  # Alias some things as locals to avoid repeat global lookups
>  cls = self.__class__
>
> maybe you should change it to self.cls??
> Try to submit a PR to the open ticket.
>
> segunda-feira, 3 de Abril de 2017 às 21:07:47 UTC+1, Kapil Garg escreveu:
>
> So does this patch seem fine ?
>
> diff --git a/django/db/models/base.py b/django/db/models/base.py
> index 3c1cb9a..f58e12b 100644
> --- a/django/db/models/base.py
> +++ b/django/db/models/base.py
> @@ -504,7 +504,7 @@ class Model(metaclass=ModelBase):
>  return '<%s: %s>' % (self.__class__.__name__, u)
>
>  def __str__(self):
> -return '%s object' % self.__class__.__name__
> +return '%s object pk=%s' % (self.__class__.__name__,
> self._get_pk_val())
>
>  def __eq__(self, other):
>  if not isinstance(other, Model):
>
>
>
>
> On Monday, 3 April 2017 23:07:56 UTC+5:30, Collin Anderson wrote:
>
> I'd recommend not saying "unsaved". "new" if anything. UUID pk's may
> default to generating a pk before save, so it might just be best to show
> the actual current pk value
>
> On Mon, Apr 3, 2017 at 1:06 PM, Kapil Garg  wrote:
>
> So what should the final __str__ show: Should it be 'ClassName object
> pk=Something' and if pk is None then should it be 'ClassName object
> (unsaved)' or 'ClassName object pk=None' ?
>
> On Sunday, 2 April 2017 23:47:01 UTC+5:30, Collin Anderson wrote:
>
> Makes sense to me. Maybe still keep the "Transaction object" part, and use
> None if no pk.
>
> On Sun, Apr 2, 2017 at 11:09 AM, Kapil Garg  wrote:
>
> Ticket 27953  is regarding
> this proposal and the suggestion is about adding "pk" in Model string
> representation if it exists.
>
> On Thursday, 11 July 2013 09:16:25 UTC+5:30, Collin Anderson wrote:
>
> Hi All,
>
> Have you ever quickly set up a model, ran syncdb, and added a few sample
> objects in the admin to only see a bunch of "MyModel object"s in the
> changelist? I always forget to add a __unicode__()/__str__() method on my
> models.
>
> I ran "git grep -1 __unicode__" on some of my django projects and noticed
> a lot of repeated code. In fact, it seems that in about a _third_ of all my
> cases, I'm just returning self.name, or returning self.name would have
> been a good default. I looked at a few 3rd party apps for comparison and
> found similar results, though not for every app.
>
> IMHO, returning self.name (if the field or property exists) is a sensible
> default for __unicode__. We can still return "MyModel object" if there's no
> "name" attribute. You'll still end up adding your own __unicode__ method
> much of the time, just like you always have.
>
> Yes, it's "magic", but we can document it.
> Yes, it's a little more confusing, but we don't have to explain it during
> the tutorial.
> Yes, it's backwards incompatible, but only in rare cases should it be a
> problem.
> Yes, it could look like any Model without a "name" field is "wrong", but
> it's not.
> Yes, "title" is also very popular, but name is better. :)
>
> It has the effect of being a little more friendly in many cases, and can
> result in more DRY code.
>
> What do your __unicode__/__str__ methods look like? Is this a bad idea?
>
> Thanks,
> Collin
>
> --
> 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-develop...@googlegroups.com.
> To post to this group, send email to django-d...@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/c7254a96-7ee3-4262-a90b-83dcfe8fa3d4%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-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at

Re: assertRaises vs. assertRaisesMessage

2017-04-07 Thread James Pic 
Without diving into implementation details, I recon I've been taught the
same as Shai. An exception type should have only one purpose, thus testing
the type /should/ be sufficient imho. That said, if you want to TDD the
message, you're going to code a test for it, is there really another way ?
;)

-- 
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/CAC6Op19xUwQQKzutqmZW73cakbgaT6%2B2-0%3Dbn6MaZgS88PTGag%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Proposal: make Model __unicode__() default to self.name

2017-04-07 Thread Marco Silva 
I noticed this on the init

def __init__(self, *args, **kwargs):
 # Alias some things as locals to avoid repeat global lookups
 cls = self.__class__

maybe you should change it to self.cls??
Try to submit a PR to the open ticket.

segunda-feira, 3 de Abril de 2017 às 21:07:47 UTC+1, Kapil Garg escreveu:
>
> So does this patch seem fine ? 
>
> diff --git a/django/db/models/base.py b/django/db/models/base.py
> index 3c1cb9a..f58e12b 100644
> --- a/django/db/models/base.py
> +++ b/django/db/models/base.py
> @@ -504,7 +504,7 @@ class Model(metaclass=ModelBase):
>  return '<%s: %s>' % (self.__class__.__name__, u)
>  
>  def __str__(self):
> -return '%s object' % self.__class__.__name__
> +return '%s object pk=%s' % (self.__class__.__name__, 
> self._get_pk_val())
>  
>  def __eq__(self, other):
>  if not isinstance(other, Model):
>
>
>
>
> On Monday, 3 April 2017 23:07:56 UTC+5:30, Collin Anderson wrote:
>>
>> I'd recommend not saying "unsaved". "new" if anything. UUID pk's may 
>> default to generating a pk before save, so it might just be best to show 
>> the actual current pk value
>>
>> On Mon, Apr 3, 2017 at 1:06 PM, Kapil Garg  wrote:
>>
>>> So what should the final __str__ show: Should it be 'ClassName object 
>>> pk=Something' and if pk is None then should it be 'ClassName object 
>>> (unsaved)' or 'ClassName object pk=None' ?
>>>
>>> On Sunday, 2 April 2017 23:47:01 UTC+5:30, Collin Anderson wrote:

 Makes sense to me. Maybe still keep the "Transaction object" part, and 
 use None if no pk.

 On Sun, Apr 2, 2017 at 11:09 AM, Kapil Garg  
 wrote:

> Ticket 27953  is 
> regarding this proposal and the suggestion is about adding "pk" in Model 
> string representation if it exists. 
>
> On Thursday, 11 July 2013 09:16:25 UTC+5:30, Collin Anderson wrote:
>>
>> Hi All,
>>
>> Have you ever quickly set up a model, ran syncdb, and added a few 
>> sample objects in the admin to only see a bunch of "MyModel object"s in 
>> the 
>> changelist? I always forget to add a __unicode__()/__str__() method on 
>> my 
>> models.
>>
>> I ran "git grep -1 __unicode__" on some of my django projects and 
>> noticed a lot of repeated code. In fact, it seems that in about a 
>> _third_ 
>> of all my cases, I'm just returning self.name, or returning self.name 
>> would have been a good default. I looked at a few 3rd party apps for 
>> comparison and found similar results, though not for every app.
>>
>> IMHO, returning self.name (if the field or property exists) is a 
>> sensible default for __unicode__. We can still return "MyModel object" 
>> if 
>> there's no "name" attribute. You'll still end up adding your own 
>> __unicode__ method much of the time, just like you always have.
>>
>> Yes, it's "magic", but we can document it.
>> Yes, it's a little more confusing, but we don't have to explain it 
>> during the tutorial.
>> Yes, it's backwards incompatible, but only in rare cases should it be 
>> a problem.
>> Yes, it could look like any Model without a "name" field is "wrong", 
>> but it's not.
>> Yes, "title" is also very popular, but name is better. :)
>>
>> It has the effect of being a little more friendly in many cases, and 
>> can result in more DRY code.
>>
>> What do your __unicode__/__str__ methods look like? Is this a bad 
>> idea?
>>
>> Thanks,
>> Collin
>>
>> -- 
> 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-develop...@googlegroups.com.
> To post to this group, send email to django-d...@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/c7254a96-7ee3-4262-a90b-83dcfe8fa3d4%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-develop...@googlegroups.com.
>>> To post to this group, send email to django-d...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/django-developers.
>>> To view this discussion on the web visit 
>>>

Re: Making __repr__ safe by default

2017-04-07 Thread Shai Berger 
On Friday 07 April 2017 15:35:55 Adam Johnson wrote:
> Patryk, when you say 'revisit', do you have links to the original
> discussion/tickets?

He was replying to a thread from 2013. You should have the link at the bottom 
of every message.

> 
> I also agree that __repr__ shouldn't be able to trigger extra DB queries in
> a standard setup.
> 

... and this ties into my other suggestion: __repr__ could be implemented as, 
basically,

with database_disabled:
return str(self)

Database "execute hooks" for instrumentation

2017-04-07 Thread Shai Berger 
Hello,

This is an idea that came up during the djangocon-europe conference: Add the 
ability to install general instrumentation hooks around the database "execute" 
and "executemany" calls.

Such hooks would allow all sorts of interesting features. For one, they could 
replace the current special-case allowing assertNumQueries & friends to record 
queries out of debug mode (it's an ugly hack, really), but they could also 
support my imagined use case -- a context-manager which could prevent database 
access during execution of some code (I'm thinking mostly of using it around 
"render()" calls and serialization, to make sure all database access is being 
done in the view).

My idea for implementation is to keep a thread-local stack of context-
managers, and have them wrap each call of "execute". We could actually even 
use one such context-manager instead of the existing CursorDebugWrapper.

Thoughts?

Re: Making __repr__ safe by default

2017-04-07 Thread Adam Johnson 
Patryk, when you say 'revisit', do you have links to the original
discussion/tickets?

I also agree that __repr__ shouldn't be able to trigger extra DB queries in
a standard setup.

On 7 April 2017 at 13:18,  wrote:

> I'd like to support this, I've seen my share of situations such as the one
> described by Patryk. Is there something we have been 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/f0e2c9b1-39d2-462e-8b8a-
> 4b1c0f4822c2%40googlegroups.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>



-- 
Adam

-- 
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/CAMyDDM1YqaxD%3DVOBshw-gLstt1_5%3DynJ%3D-wKe_sQazYmRKXyfg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: assertRaises vs. assertRaisesMessage

2017-04-07 Thread Adam Johnson 
That said more structured attributes on exceptions can be useful for Django
users, if you know any obvious places they are missing.

On 7 April 2017 at 13:23, Adam Johnson  wrote:

> I agree with Tim. He's also probably the most familiar with the cost of
> message changes 
>
> On 7 April 2017 at 12:56, Tim Graham  wrote:
>
>> Hi Shai,
>>
>> Without testing for a message, there's also a possibility that the
>> exception you think you're testing isn't the one the test is actually
>> raising. I think the readability advantages of including messages in the
>> tests outweigh the cost of updating tests when a message changes.
>>
>>
>> On Friday, April 7, 2017 at 5:30:25 AM UTC-4, Shai Berger wrote:
>>>
>>> Hello all,
>>>
>>> A recent PR[1] seeks to replace most of the assertRaises() calls in the
>>> test-
>>> suite with assertRaisesMessage(). The argument for it is that it then
>>> becomes
>>> easier to find the test for some error handling, by grepping the test
>>> for the
>>> message text. Also, in some cases, an existing assertRaises() is
>>> followed by a
>>> separate, explicit verification of the message text[2], which surely
>>> indicates
>>> that assertRaisesMessage() should have been used; and actually, this
>>> whole
>>> move initiated as part of the Py2-support-deprecation changes.
>>>
>>> However, I don't think we should generally prefer assertRaisesMessage to
>>> assertRaises, because it means that trivial phrasing changes need to be
>>> made
>>> twice (at least) instead of once. I don't believe exception messages
>>> should be
>>> considered part of the stable API (as opposed to exception types), and
>>> so, I
>>> think it is, in many cases, actually wrong to test them as such.
>>>
>>> In some places, it makes sense to assert that certain information is
>>> included
>>> in the message, and then assertRaisesRegex could be used. Tim made the
>>> practical argument that regexes in these circumstances introduce more
>>> complexity than they're worth, and that is certainly a valid argument; I
>>> can
>>> agree with a general policy of "prefer assertRaisesMessage to
>>> assertRaisesRegex". But I think in most places, we should only test for
>>> the
>>> exception type, and if more info is required, it should be structured --
>>> that
>>> is, presented as attributes on the exception object rather than text in
>>> the
>>> message.
>>>
>>> What do you think?
>>>
>>> Thanks,
>>> Shai.
>>>
>>> [1] https://github.com/django/django/pull/8257
>>> [2] https://github.com/django/django/pull/8257#pullrequestreview
>>> -30486470
>>>
>> --
>> 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/ms
>> gid/django-developers/e1e8010e-903f-45dd-8e15-9f5da8150665%
>> 40googlegroups.com
>> 
>> .
>>
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>
>
> --
> Adam
>



-- 
Adam

-- 
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/CAMyDDM001ba%3DCePkwcW9QwZdNNHGPtJQA1361SW_vW9hSF4%3Dyw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: assertRaises vs. assertRaisesMessage

2017-04-07 Thread Adam Johnson 
I agree with Tim. He's also probably the most familiar with the cost of
message changes 

On 7 April 2017 at 12:56, Tim Graham  wrote:

> Hi Shai,
>
> Without testing for a message, there's also a possibility that the
> exception you think you're testing isn't the one the test is actually
> raising. I think the readability advantages of including messages in the
> tests outweigh the cost of updating tests when a message changes.
>
>
> On Friday, April 7, 2017 at 5:30:25 AM UTC-4, Shai Berger wrote:
>>
>> Hello all,
>>
>> A recent PR[1] seeks to replace most of the assertRaises() calls in the
>> test-
>> suite with assertRaisesMessage(). The argument for it is that it then
>> becomes
>> easier to find the test for some error handling, by grepping the test for
>> the
>> message text. Also, in some cases, an existing assertRaises() is followed
>> by a
>> separate, explicit verification of the message text[2], which surely
>> indicates
>> that assertRaisesMessage() should have been used; and actually, this
>> whole
>> move initiated as part of the Py2-support-deprecation changes.
>>
>> However, I don't think we should generally prefer assertRaisesMessage to
>> assertRaises, because it means that trivial phrasing changes need to be
>> made
>> twice (at least) instead of once. I don't believe exception messages
>> should be
>> considered part of the stable API (as opposed to exception types), and
>> so, I
>> think it is, in many cases, actually wrong to test them as such.
>>
>> In some places, it makes sense to assert that certain information is
>> included
>> in the message, and then assertRaisesRegex could be used. Tim made the
>> practical argument that regexes in these circumstances introduce more
>> complexity than they're worth, and that is certainly a valid argument; I
>> can
>> agree with a general policy of "prefer assertRaisesMessage to
>> assertRaisesRegex". But I think in most places, we should only test for
>> the
>> exception type, and if more info is required, it should be structured --
>> that
>> is, presented as attributes on the exception object rather than text in
>> the
>> message.
>>
>> What do you think?
>>
>> Thanks,
>> Shai.
>>
>> [1] https://github.com/django/django/pull/8257
>> [2] https://github.com/django/django/pull/8257#pullrequestreview-30486470
>>
> --
> 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/e1e8010e-903f-45dd-8e15-
> 9f5da8150665%40googlegroups.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>



-- 
Adam

-- 
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/CAMyDDM3ySnN3u58uRkXrHpJnmpzBXzQLjnj0t13di%3DxyaSM0Bw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: Making __repr__ safe by default

2017-04-07 Thread jpic 
I'd like to support this, I've seen my share of situations such as the one 
described by Patryk. Is there something we have been 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/f0e2c9b1-39d2-462e-8b8a-4b1c0f4822c2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: assertRaises vs. assertRaisesMessage

2017-04-07 Thread Shai Berger 
Hi Tim,

> Without testing for a message, there's also a possibility that the
> exception you think you're testing isn't the one the test is actually
> raising. 

If you need to verify the error message for that reason, then the exception is 
probably not specific enough. Testing the message, in this case, is just 
working around faulty design.

> I think the readability advantages of including messages in the
> tests outweigh the cost of updating tests when a message changes.

In some cases, of course; but I see a significant distance between that and a 
general recommendation to prefer assertRaisesMessage.

Shai.

Re: assertRaises vs. assertRaisesMessage

2017-04-07 Thread Tim Graham 
Hi Shai,

Without testing for a message, there's also a possibility that the 
exception you think you're testing isn't the one the test is actually 
raising. I think the readability advantages of including messages in the 
tests outweigh the cost of updating tests when a message changes.

On Friday, April 7, 2017 at 5:30:25 AM UTC-4, Shai Berger wrote:
>
> Hello all, 
>
> A recent PR[1] seeks to replace most of the assertRaises() calls in the 
> test- 
> suite with assertRaisesMessage(). The argument for it is that it then 
> becomes 
> easier to find the test for some error handling, by grepping the test for 
> the 
> message text. Also, in some cases, an existing assertRaises() is followed 
> by a 
> separate, explicit verification of the message text[2], which surely 
> indicates 
> that assertRaisesMessage() should have been used; and actually, this whole 
> move initiated as part of the Py2-support-deprecation changes. 
>
> However, I don't think we should generally prefer assertRaisesMessage to 
> assertRaises, because it means that trivial phrasing changes need to be 
> made 
> twice (at least) instead of once. I don't believe exception messages 
> should be 
> considered part of the stable API (as opposed to exception types), and so, 
> I 
> think it is, in many cases, actually wrong to test them as such. 
>
> In some places, it makes sense to assert that certain information is 
> included 
> in the message, and then assertRaisesRegex could be used. Tim made the 
> practical argument that regexes in these circumstances introduce more 
> complexity than they're worth, and that is certainly a valid argument; I 
> can 
> agree with a general policy of "prefer assertRaisesMessage to 
> assertRaisesRegex". But I think in most places, we should only test for 
> the 
> exception type, and if more info is required, it should be structured -- 
> that 
> is, presented as attributes on the exception object rather than text in 
> the 
> message. 
>
> What do you think? 
>
> Thanks, 
> Shai. 
>
> [1] https://github.com/django/django/pull/8257 
> [2] https://github.com/django/django/pull/8257#pullrequestreview-30486470 
>

-- 
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/e1e8010e-903f-45dd-8e15-9f5da8150665%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Making __repr__ safe by default

2017-04-07 Thread Patryk Zawadzki 
Hey, it's been a number of years and I'd like to revisit this topic.

I still consider it an anti-pattern to have __repr__ make a call to 
self.__unicode__ or self.__str__ as these can (and very often do) refer to 
related database objects. The last thing you want to happen while debugging 
a crash is for the mere act of observation to result in more database 
queries being executed. __repr__ is assumed to be safe to call by most 
debugging tools and is called by all crash reporting tools I've worked 
with. Having __repr__ itself crash (for example when the database 
connection is invalid) while handling an exception in a production 
environment can lead to exceptions being silently ignored or to 
misleading/useless stack traces being reported.

PS: Backwards compatibility was brought up as a potential issue but 
__repr__ is not usually an API consumed by application code.

Cheers

-- 
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/097746a7-2eec-4390-9f29-200a5213b9dc%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [feature request] including HttpResponse(status=204) as an HttpResponse subclasses

2017-04-07 Thread Berker Peksağ 
On Fri, Apr 7, 2017 at 9:54 AM, Adam Johnson  wrote:
> Personally I'd be in favour of adding such classes. It seems against the
> batteries-included philosophy that Django does not provide all of the
> standard codes as classes. I can never remember which codes correspond to
> which response types, if I saw status=204 in code it would be a 'magic
> number' for me and I'd have to look it up. HttpResponseRedirect and
> HttpResponsePermanentRedirect have been my friends in the past, I can
> imagine the same for HttpResponseNoContent.

Alternatively, they can use the HTTPStatus enum from the stdlib if
they are on Python 3.5+:

from http import HTTPStatus

HttpResponse(status=HTTPStatus.NO_CONTENT)

--Berker

-- 
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/CAF4280K-fT8bRnc6fvpZj0RjuhK8b2-eSozMKEJMsPT0EinevQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

assertRaises vs. assertRaisesMessage

2017-04-07 Thread Shai Berger 
Hello all,

A recent PR[1] seeks to replace most of the assertRaises() calls in the test-
suite with assertRaisesMessage(). The argument for it is that it then becomes 
easier to find the test for some error handling, by grepping the test for the 
message text. Also, in some cases, an existing assertRaises() is followed by a 
separate, explicit verification of the message text[2], which surely indicates 
that assertRaisesMessage() should have been used; and actually, this whole 
move initiated as part of the Py2-support-deprecation changes.

However, I don't think we should generally prefer assertRaisesMessage to 
assertRaises, because it means that trivial phrasing changes need to be made 
twice (at least) instead of once. I don't believe exception messages should be 
considered part of the stable API (as opposed to exception types), and so, I 
think it is, in many cases, actually wrong to test them as such.

In some places, it makes sense to assert that certain information is included 
in the message, and then assertRaisesRegex could be used. Tim made the 
practical argument that regexes in these circumstances introduce more 
complexity than they're worth, and that is certainly a valid argument; I can 
agree with a general policy of "prefer assertRaisesMessage to 
assertRaisesRegex". But I think in most places, we should only test for the 
exception type, and if more info is required, it should be structured -- that 
is, presented as attributes on the exception object rather than text in the 
message.

What do you think?

Thanks,
Shai.

[1] https://github.com/django/django/pull/8257
[2] https://github.com/django/django/pull/8257#pullrequestreview-30486470

-- 
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/201704071230.10562.shai%40platonix.com.
For more options, visit https://groups.google.com/d/optout.

Re: [feature request] including HttpResponse(status=204) as an HttpResponse subclasses

2017-04-07 Thread Marc Tamlyn 
I would be happy to revisit that decision which was made 9 years ago. APIs
returning unusual status codes such as 204 are much more common now than
they were then. I can't think of a good reason not to add ~10-15 2-line
classes.

Marc

On 7 April 2017 at 09:37, Brice PARENT  wrote:

>
> Le 07/04/17 à 08:54, Adam Johnson a écrit :
>
> Personally I'd be in favour of adding such classes. It seems against the
> batteries-included philosophy that Django does not provide all of the
> standard codes as classes. I can never remember which codes correspond to
> which response types, if I saw status=204 in code it would be a 'magic
> number' for me and I'd have to look it up. HttpResponseRedirect and H
> ttpResponsePermanentRedirect have been my friends in the past, I can
> imagine the same for HttpResponseNoContent.
>
> +1
>
> And there are more than just HttpResponseRedirect and 
> HttpResponsePermanentRedirect
> that are provided by now.
>
> In django.http.response, you can find :
>
> HttpResponseRedirect, HttpResponsePermanentRedirect,
> HttpResponseNotModified, HttpResponseBadRequest, HttpResponseNotFound,
> HttpResponseForbidden, HttpResponseNotAllowed, HttpResponseGone,
> HttpResponseServerError
>
> of which 7 don't embed any code, just the status code (like what is asked
> for 204).
>
> I'm not saying all of them are widely used enough to require an explicit
> class declaration (like status code 418) or even would mean anything in the
> context of a web framework, but the most common should, at least, be in the
> batteries. 204 is an example, but there probably are some others (like 426
> and 505, when the world is switching to https).
>
> -OR-
>
> We should at least provide a reversed version of django.http.response.
> REASON_PHRASES (which doesn't exist anymore, I think), so that we could
> call :
>
> HttpResponse(status=response_codes.no_content)
>
> It makes it way less cryptic for both the writer and the readers of the
> code.
>
> -Brice
>
> --
> 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/3d69e4e0-0f4a-3f75-3315-3b41c7569ffb%40brice.xyz
> 
> .
>
> 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/CAMwjO1F6qCBTyVhEN0MbkSh2fHJNt%3DFVyMTmQ%3Dn_Knt23HVPZA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: [feature request] including HttpResponse(status=204) as an HttpResponse subclasses

2017-04-07 Thread Adam Johnson 
Personally I'd be in favour of adding such classes. It seems against the
batteries-included philosophy that Django does not provide all of the
standard codes as classes. I can never remember which codes correspond to
which response types, if I saw status=204 in code it would be a 'magic
number' for me and I'd have to look it up. HttpResponseRedirect and
HttpResponsePermanentRedirect have been my friends in the past, I can
imagine the same for HttpResponseNoContent.

On 7 April 2017 at 00:44, Philip Lee  wrote:

> I think you'd better write the decision in the document to stop Django
> users from making the same feature request in future !
>
> On Thursday, April 6, 2017 at 12:35:41 AM UTC+8, Tim Graham wrote:
>>
>> Hi, this was already wontfixed here:
>> https://code.djangoproject.com/ticket/3362
>>
>> with the rationale, "We've decided in the past not to add a new class for
>> every single response code. You can already pass the status code in when
>> creating the HttpResponse class, so that can be used in this case."
>>
>> (found with this google search: httpresponse 204 site:
>> code.djangoproject.com)
>>
>> On Wednesday, April 5, 2017 at 11:25:44 AM UTC-4, Philip Lee wrote:
>>>
>>> Every Django view function MUST return an HttpResponse object, sometimes we 
>>> just want to send data to the server and  don't want to send data back to 
>>> the client , this probably lead Python users want to return None instead, 
>>> however , this is not allowed in Django view function, I found  
>>> HttpResponse(status=204) 
>>>  may come 
>>> for rescue in this case,  thus it would be better to include 
>>> HttpResponse(status=204) as an HttpResponse subclasses for convenience so 
>>> that could save Django users from asking those returning-null-response 
>>> questions:http://stackoverflow.com/questions/17557618/post-without-response-in-django-javascript-interactionhttp://stackoverflow.com/questions/2131203/django-no-redirectionshttp://stackoverflow.com/questions/4123155/how-do-i-send-empty-response-in-django-without-templates
>>>
>>> BTW, someone already implemented this feature  here, better to adopt it in 
>>> Django http://django-extras.readthedocs.io/en/latest/ref/http-response.html
>>>
>>> --
> 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/2f619780-d6f1-4bce-8d34-
> 40a18b87d798%40googlegroups.com
> 
> .
>
> For more options, visit https://groups.google.com/d/optout.
>



-- 
Adam

-- 
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/CAMyDDM3mrxWg4tW6CTYKMCxU4Fsk8Fs55uEVtEN6gR-yAw2QOw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.