Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

FYI: I opened a thread on Python-ideas where we continued the discussion on
my `raise as` proposal, Shai's proposal, etc.:
https://mail.python.org/archives/list/python-id...@python.org/thread/KM7NRNFZHALOBKJUXVYQL2SLDP3MAANW/

On Fri, Feb 7, 2020 at 1:16 PM Ram Rachum  wrote:

>
>
> On Fri, Feb 7, 2020 at 12:23 PM Carlton Gibson 
> wrote:
>
>> > I'm basing it on the fact that Carlton approved this PR for the style
>> guide: https://github.com/django/django/pull/12350
>>
>> No. I don't think we should merge that change. (It's "approved" qua
>> itself before reviewing, and dependent on the main PR.)
>>
>> To be clear. I think the default implicit chaining should be used unless
>> there's specific reason not to.
>>
>> (In hindsight I should have come to this conclusion before agreeing to
>> the original smaller cleanup.)
>>
>> C.
>>
>
> I understand. Hypothetical question: If the syntax was `raise as
> NewException`, without having to give the old exception a name, would that
> change your decision?
>

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

Re: Use "raise from" where appropriate, all over the codebase

[Carlton Gibson]

Maybe... it's still more verbose for no gain as I see it. 

I think the default implicit chaining is correct in the default case. It's 
only if you want to adjust that (or suppress is with `from None`) that the 
extra clause comes in handy. I think using the default unless there's a 
reason not to is, in general, a good policy. 

I know I'm -1 on this particular change, for the reasons in this thread, 
but thank you for your efforts nonetheless. :)

Kind Regards,

Carlton


On Friday, 7 February 2020 12:17:11 UTC+1, Ram Rachum wrote:
>
>
>
> On Fri, Feb 7, 2020 at 12:23 PM Carlton Gibson  > wrote:
>
>> > I'm basing it on the fact that Carlton approved this PR for the style 
>> guide: https://github.com/django/django/pull/12350
>>
>> No. I don't think we should merge that change. (It's "approved" qua 
>> itself before reviewing, and dependent on the main PR.)
>>
>> To be clear. I think the default implicit chaining should be used unless 
>> there's specific reason not to. 
>>
>> (In hindsight I should have come to this conclusion before agreeing to 
>> the original smaller cleanup.)
>>
>> C.
>>
>
> I understand. Hypothetical question: If the syntax was `raise as 
> NewException`, without having to give the old exception a name, would that 
> change your decision?
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/0bef0533-553c-41a4-b11a-4346dcc17c83%40googlegroups.com.

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

On Fri, Feb 7, 2020 at 12:23 PM Carlton Gibson 
wrote:

> > I'm basing it on the fact that Carlton approved this PR for the style
> guide: https://github.com/django/django/pull/12350
>
> No. I don't think we should merge that change. (It's "approved" qua itself
> before reviewing, and dependent on the main PR.)
>
> To be clear. I think the default implicit chaining should be used unless
> there's specific reason not to.
>
> (In hindsight I should have come to this conclusion before agreeing to the
> original smaller cleanup.)
>
> C.
>

I understand. Hypothetical question: If the syntax was `raise as
NewException`, without having to give the old exception a name, would that
change your decision?

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

Re: Use "raise from" where appropriate, all over the codebase

[Carlton Gibson]

> I'm basing it on the fact that Carlton approved this PR for the style 
guide: https://github.com/django/django/pull/12350

No. I don't think we should merge that change. (It's "approved" qua itself 
before reviewing, and dependent on the main PR.)

To be clear. I think the default implicit chaining should be used unless 
there's specific reason not to. 

(In hindsight I should have come to this conclusion before agreeing to the 
original smaller cleanup.)

C.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9ad99f34-f2bc-407f-9bef-8f9cdaa21e5c%40googlegroups.com.

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

On Fri, Feb 7, 2020 at 12:27 AM Aymeric Augustin <
aymeric.augus...@polytechnique.org> wrote:

> Hello Ram,
>
> On 6 Feb 2020, at 19:08, Ram Rachum  wrote:
>
> In other words, "raise from" is the inevitable future, it's just that
> we're not in a rush to get there.
>
>
> I'm not sure how you came to this conclusion; I'm not seeing this in
> Carlton's and Mariusz' answers.
>

I'm basing it on the fact that Carlton approved this PR for the style
guide: https://github.com/django/django/pull/12350

I'm not sure though whether he intends to merge it.

In any case, I think it'll be sad if people will just get used to
misleading exception chaining messages.

Shai Berger wrote:
> > > it's very rare to have a legitimate exception
> > > without a "raise from" inside an except clause. In almost any
> > > context in which "during handling of..." is correct, the raising is
> > > done deeper in the stack.
> > >
> I think the conclusion should be to ask for a change in Python, not
> Django. The rule "if an exception is raised explicitly from an except
> clause then it is considered raised-from" seems simple enough to me.

That's an interesting idea, but there are 2-3 problems with it that make it
close to impossible.

An alternative suggestion would be to allow syntax that automatically sets
the cause to be the last-caught exception. I would suggest the `raise as`
syntax, which was one of the rejected alternatives in PEP 409
 for a different
usecase

try:
1/0

except ZeroDivisionError:

raise as ValueError('Whatever')

My suggestion is that this `raise as` line would have the same effect as
`raise ValueError('Whatever') from zero_division_error`, while also
preventing the need to add `as zero_division_error` in the line above.

I think this solves most of the problems with the previous suggestion. What
do you think? Want to write a PEP together? :)

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

Re: Use "raise from" where appropriate, all over the codebase

[Ryan Hiebert]

>
> I think the conclusion should be to ask for a change in Python, not
> Django. The rule "if an exception is raised explicitly from an except
> clause then it is considered raised-from" seems simple enough to me.
>

I really like that. It makes perfect sense, and I can't think of a case
where that rule would be incorrect.

>

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

Re: Use "raise from" where appropriate, all over the codebase

[Shai Berger]

On Thu, 6 Feb 2020 20:08:28 +0200
Ram Rachum  wrote:

> 
> If I understand correctly, you both agree that using "raise from" in
> this context is better than using plain raise, just that the benefits
> are not worth the price of a bulk update to Django. In other words,
> "raise from" is the inevitable future, it's just that we're not in a
> rush to get there.

As Aymeric pointed out, that's inaccurate; If I understood correctly,
Carlton's argument amounted to "it would be better to have the string
produced by `raise from` in the output, but it's not worth the extra
verbosity in exception handling code" -- besides the price of the bulk
update.

> Keep in mind that the thing I care about is not just usage of "raise
> from" in Django, but in any Python package which is relevant.

... and Carlton's argument applies to them all; when also considering:

> On Sat, 18 Jan 2020 17:18:41 +0200
> Ram Rachum  wrote:
> 
> > 
> > it's very rare to have a legitimate exception
> > without a "raise from" inside an except clause. In almost any
> > context in which "during handling of..." is correct, the raising is
> > done deeper in the stack.
> > 

I think the conclusion should be to ask for a change in Python, not
Django. The rule "if an exception is raised explicitly from an except
clause then it is considered raised-from" seems simple enough to me.

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

Re: Use "raise from" where appropriate, all over the codebase

[Aymeric Augustin]

Hello Ram,

> On 6 Feb 2020, at 19:08, Ram Rachum  wrote:
> 
> In other words, "raise from" is the inevitable future, it's just that we're 
> not in a rush to get there.


I'm not sure how you came to this conclusion; I'm not seeing this in Carlton's 
and Mariusz' answers.

Carlton only said that `raise from` can be useful in specific cases where the 
default, implicit exception chaining doesn't do what you need.

> On 6 Feb 2020, at 15:37, Carlton Gibson  wrote:
> 
> Beyond that, the from syntax is simply more verbose. In the PR the changes 
> are 
> all adding a unneeded `as e`, followed by an equally unneeded `from e`. 


This wouldn't be an improvement.

Best regards,

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/5AF3A99A-4C6E-4919-BACB-7FFAE7C34FEC%40polytechnique.org.

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

Hi guys,

I'm disappointed that you're against this change... But I understand that
you have a different perspective. Here's my last-ditch effort to convince
you.

If I understand correctly, you both agree that using "raise from" in this
context is better than using plain raise, just that the benefits are not
worth the price of a bulk update to Django. In other words, "raise from" is
the inevitable future, it's just that we're not in a rush to get there.

Keep in mind that the thing I care about is not just usage of "raise from"
in Django, but in any Python package which is relevant. Most Python
developers don't know about this difference between "During the handling
of" and "The above exception was the direct cause", because "raise from"
isn't that prevalent yet. This is a chicken-and-egg problem, because as
long as project maintainers don't use "raise from", the exception chaining
wouldn't show the correct text, and people would get used to the fact that
they can't rely on this text to be correct.

Like any chicken-and-egg problem of changing people's habits, the best we
could hope is to move it forward at a glacial pace-- A situation somewhat
similar to the move to Python 3. If Django were to adopt this practice, it
would help in getting other projects to do that too, and for people to pay
attention to that line of text.


Thanks,
Ram.

On Thu, Feb 6, 2020 at 4:48 PM Mariusz Felisiak 
wrote:

> I agree with Carlton, I don't see much (if any) value in the bulk update.
>
> Best,
> Mariusz
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/ibEOt3A9c2M/unsubscribe
> .
> To unsubscribe from this group and all its topics, send an email to
> django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/ab6abbb0-f651-4271-b84c-a3bacaba9482%40googlegroups.com
> 
> .
>

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

Re: Use "raise from" where appropriate, all over the codebase

[Mariusz Felisiak]

I agree with Carlton, I don't see much (if any) value in the bulk update.

Best,
Mariusz

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

Re: Use "raise from" where appropriate, all over the codebase

[Carlton Gibson]

> +1 on chaining exceptions. I think the information is useful.

Absolutely. But exceptions are **already** chained, regardless of whether 
we 
use the `from` syntax. 

Without the from clause exceptions are "implicitly" chained. With the from 
claus it's "explicit". 

Just the same tracebacks are presented, in just the same order. 

The only difference is the string that divides them: 

```
The above exception was the direct cause of the following exception:
```

vs

```
During handling of the above exception, another exception occurred:
```

That change in string isn't worth 380+ changes over 100 files. 

* We're obscuring the history for nothing, and...
* Whilst I **think** there's no danger in this case, bulk edits are by 
their nature 
  unconsidered, and there is the ever-present risk of regression. 
  
Beyond that, the from syntax is simply more verbose. In the PR the changes 
are 
all adding a unneeded `as e`, followed by an equally unneeded `from e`. 

Looking at the diff, the comparison that comes to mind is if we went 
through 
and replaced all percent formatting with format()



So I'm -1 on merging the PR. I think we should close the ticket as wontfix. 

I would ask everyone to clarify exactly what they think the benefits of the 
proposed change are, and to follow-up if there's a hidden benefit I've 
missed. 


I'm not anti the from clause in general. I use it myself, and in any given, 
considered case, I see no reason not to use it in Django. 

This is different from insisting it must be used, and rightly so, because, 
as I understand it, the target use case for from is when you want to 
provide more targeted exception logging. 

Take this case: 

def inner():
raise Exception("This is the one we really want to show the user.")


def middle():
try:
inner()
except:
raise Exception("All sorts of stuff that's not so interesting.")


def outer():
try:
middle()
except Exception as e:
msg = "We cut out the unimportant stuff, to focus on what's 
important."
raise Exception(msg) from e.__context__


if __name__ == '__main__':
outer()

Here the use of from makes the traceback exactly how we want it:

$ python exceptions.py 
Traceback (most recent call last):
  File "exceptions.py", line 9, in middle
inner()
  File "exceptions.py", line 4, in inner
raise Exception("This is the one we really want to show the user.")
Exception: This is the one we really want to show the user.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "exceptions.py", line 23, in 
outer()
  File "exceptions.py", line 19, in outer
raise Exception(msg) from e.__context__
Exception: We cut out the unimportant stuff, to focus on what's 
important.
 
Contrasted to not using from, where we just get everything: 

$ python exceptions.py 
Traceback (most recent call last):
  File "exceptions.py", line 9, in middle
inner()
  File "exceptions.py", line 4, in inner
raise Exception("This is the one we really want to show the user.")
Exception: This is the one we really want to show the user.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "exceptions.py", line 16, in outer
middle()
  File "exceptions.py", line 11, in middle
raise Exception("All sorts of stuff that's not so interesting.")
Exception: All sorts of stuff that's not so interesting.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "exceptions.py", line 23, in 
outer()
  File "exceptions.py", line 19, in outer
raise Exception(msg)
Exception: We just output everything because we didn't use from.
 
That's where the real benefit lies. 

So, summary: 

* Exception chaining is the default. We already have that. 
* Short of something I missed (which is always possible ) there's no 
reason for the bulk edit being proposed here. 

Kind Regards,

Carlton

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/8fd6f226-7ee6-403b-9a8e-9d02408a87c2%40googlegroups.com.

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

I made a pull request for the style guide if anyone would like to review:
https://github.com/django/django/pull/12350

On Mon, Jan 20, 2020 at 6:05 PM Ram Rachum  wrote:

> Jon: That's awesome! I also liked R101. I didn't think of that.
>
> Adam: I thought so too, but after going over dozens of R100 cases, I
> didn't find even one where a raise without "from" inside an except clause
> was justified. I challenge you to show me even one such example.
>
> On Mon, Jan 20, 2020 at 12:06 PM Adam Johnson  wrote:
>
>> Nice work Jon. I don't think we can generally apply R100 though - thre
>> are definitely cases where "raise" inside an exception handler is not
>> *caused* by the other exception - but maybe that's a sign refactoring is
>> needed.
>>
>> On Sun, 19 Jan 2020 at 22:52, Jon Dufresne 
>> wrote:
>>
>>>
>>> > I think it's rare enough that personally, I would have liked to have a
 > Flake8 / PyLint rule like that enforces it, and allow ignoring it
 > with a comment. (As much as I hate Lint ignore comments.)

 That makes sense. And you know, flake8 supports plugins... a couple of
 web searches and "pip search flake8 | grep {raise,from,chain}" didn't
 pull anything which seems relevant, but if so, it can be written.

>>>
>>> As a weekend project, I wrote a flake8 plugin to handle this at:
>>> https://pypi.org/project/flake8-raise/
>>>
>>> I'm not advocating this necessarily included by the Django test suite,
>>> but it can be used to review the PR. Either way, it was fun to write.
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to django-developers+unsubscr...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/django-developers/CADhq2b4m40efQkorgji9-erXri9qSAb1VJBv%3DZ3HrOL6ksOJAQ%40mail.gmail.com
>>> 
>>> .
>>>
>>
>>
>> --
>> 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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CANXboVbjPhmAK-p%3DCmNACu%2BX5bnLt7gW7X9wVBXJvQSdHnLvsg%40mail.gmail.com.

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

Jon: That's awesome! I also liked R101. I didn't think of that.

Adam: I thought so too, but after going over dozens of R100 cases, I didn't
find even one where a raise without "from" inside an except clause was
justified. I challenge you to show me even one such example.

On Mon, Jan 20, 2020 at 12:06 PM Adam Johnson  wrote:

> Nice work Jon. I don't think we can generally apply R100 though - thre are
> definitely cases where "raise" inside an exception handler is not *caused*
> by the other exception - but maybe that's a sign refactoring is needed.
>
> On Sun, 19 Jan 2020 at 22:52, Jon Dufresne  wrote:
>
>>
>> > I think it's rare enough that personally, I would have liked to have a
>>> > Flake8 / PyLint rule like that enforces it, and allow ignoring it
>>> > with a comment. (As much as I hate Lint ignore comments.)
>>>
>>> That makes sense. And you know, flake8 supports plugins... a couple of
>>> web searches and "pip search flake8 | grep {raise,from,chain}" didn't
>>> pull anything which seems relevant, but if so, it can be written.
>>>
>>
>> As a weekend project, I wrote a flake8 plugin to handle this at:
>> https://pypi.org/project/flake8-raise/
>>
>> I'm not advocating this necessarily included by the Django test suite,
>> but it can be used to review the PR. Either way, it was fun to write.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/CADhq2b4m40efQkorgji9-erXri9qSAb1VJBv%3DZ3HrOL6ksOJAQ%40mail.gmail.com
>> 
>> .
>>
>
>
> --
> 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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CANXboVYGg33WfP0jLWNPkKLBeL49gnbf%3DP5BM12NUPNzMeNJyg%40mail.gmail.com.

Re: Use "raise from" where appropriate, all over the codebase

[Adam Johnson]

Nice work Jon. I don't think we can generally apply R100 though - thre are
definitely cases where "raise" inside an exception handler is not *caused*
by the other exception - but maybe that's a sign refactoring is needed.

On Sun, 19 Jan 2020 at 22:52, Jon Dufresne  wrote:

>
> > I think it's rare enough that personally, I would have liked to have a
>> > Flake8 / PyLint rule like that enforces it, and allow ignoring it
>> > with a comment. (As much as I hate Lint ignore comments.)
>>
>> That makes sense. And you know, flake8 supports plugins... a couple of
>> web searches and "pip search flake8 | grep {raise,from,chain}" didn't
>> pull anything which seems relevant, but if so, it can be written.
>>
>
> As a weekend project, I wrote a flake8 plugin to handle this at:
> https://pypi.org/project/flake8-raise/
>
> I'm not advocating this necessarily included by the Django test suite, but
> it can be used to review the PR. Either way, it was fun to write.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CADhq2b4m40efQkorgji9-erXri9qSAb1VJBv%3DZ3HrOL6ksOJAQ%40mail.gmail.com
> 
> .
>


-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM0c3J%2BRGZZB%3D-Er3fAYnyvptkwu34CaO%2BmA%2BmKPKTEYPw%40mail.gmail.com.

Re: Use "raise from" where appropriate, all over the codebase

[Jon Dufresne]

> > I think it's rare enough that personally, I would have liked to have a
> > Flake8 / PyLint rule like that enforces it, and allow ignoring it
> > with a comment. (As much as I hate Lint ignore comments.)
>
> That makes sense. And you know, flake8 supports plugins... a couple of
> web searches and "pip search flake8 | grep {raise,from,chain}" didn't
> pull anything which seems relevant, but if so, it can be written.
>

As a weekend project, I wrote a flake8 plugin to handle this at:
https://pypi.org/project/flake8-raise/

I'm not advocating this necessarily included by the Django test suite, but
it can be used to review the PR. Either way, it was fun to write.

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

Re: Use "raise from" where appropriate, all over the codebase

[Adam Johnson]

>
> I would like to make the point that chained exceptions might be slightly
> annoying when displayed via console output, as you see the inner exception
> first and have to scroll up to see the exception you actually have to
> handle.


Just coming back to this, Tom it's not quite true. Yes you see the "inner"
exception first - meaning the one raised inside the 'except' block. But
that is the one you, as a user, have to handle. This is not a concern for
me.

I checked with a simple program that Python prints them in the same order
and format whether or not "raise ... from" is used on the inner. The only
change is the message reads "The above exception was the direct cause of
the following exception:" which is pretty clear I think.

On Sat, 18 Jan 2020 at 15:18, Ram Rachum  wrote:

>
>
> On Sat, Jan 18, 2020 at 5:05 PM Shai Berger  wrote:
>
>> [snip]
>
> But as it turns out, `from` puts the
>> original exception on the `__cause__` in *addition* to `__context__`:
>>
>> [snip]
>> So that is not a concern.
>>
>
> Awesome! I did not know that.
>
>
>
>> > Regarding automatically enforcing this format going forward: I looked
>> > at the list of Flake8 rules  and
>> > couldn't find anything about it.
>> >
>>
>> Indeed, I don't think there can be -- it cannot be a style violation to
>> run into an error while handling another error...
>>
>
> I think it can be, as it's very rare to have a legitimate exception
> without a "raise from" inside an except clause. In almost any context in
> which "during handling of..." is correct, the raising is done deeper in the
> stack.
>
> I think it's rare enough that personally, I would have liked to have a
> Flake8 / PyLint rule like that enforces it, and allow ignoring it with a
> comment. (As much as I hate Lint ignore comments.)
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CANXboVYxb%2BMeOHp%3De4gzqroKWkHzLjeGXy55qJyRLQQVcUPrWg%40mail.gmail.com
> 
> .
>


-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM0%2BcpfiqFs3X9Kw9g%3D7444JahSjGBCa%2B2piSm71g_%3D12g%40mail.gmail.com.

Re: Use "raise from" where appropriate, all over the codebase

[Shai Berger]

On Sat, 18 Jan 2020 17:18:41 +0200
Ram Rachum  wrote:

> On Sat, Jan 18, 2020 at 5:05 PM Shai Berger  wrote:
> 
> > > Regarding automatically enforcing this format going forward: I
> > > looked at the list of Flake8 rules 
> > > and couldn't find anything about it.
> > >  
> >
> > Indeed, I don't think there can be -- it cannot be a style
> > violation to run into an error while handling another error...
> >  
> 
> I think it can be, as it's very rare to have a legitimate exception
> without a "raise from" inside an except clause. In almost any context
> in which "during handling of..." is correct, the raising is done
> deeper in the stack.
> 
> I think it's rare enough that personally, I would have liked to have a
> Flake8 / PyLint rule like that enforces it, and allow ignoring it
> with a comment. (As much as I hate Lint ignore comments.)

That makes sense. And you know, flake8 supports plugins... a couple of
web searches and "pip search flake8 | grep {raise,from,chain}" didn't
pull anything which seems relevant, but if so, it can be written.

Shai.

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

Re: Use "raise from" where appropriate, all over the codebase

[Jon Dufresne]

+1 on chaining exceptions. I think the information is useful.

> Is there anything we can do to control the way python displays them?

I don't think we should do anything non-standard to display exceptions.
Over time, Python programmers have become accustomed to how these
exceptions are displayed. To alter that would be to introduce something
unfamiliar from all their other Python debugging experiences.

If exception displaying should be improved, I think the effort should be
upstream to benefit the larger Python community.

> And how would we ensure the format is kept going forwards? Is there a
flake8 rule/plugin we could activate to enforce it?

At the very least, we could add a note in the code style guidelines that
chained exceptions should normally be used.

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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CADhq2b41caYYAXS%3DypOs6wCQ%3D%2BcSE%3D53Uxk6ia4009ZOo8XiNA%40mail.gmail.com.

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

On Sat, Jan 18, 2020 at 5:05 PM Shai Berger  wrote:

> [snip]

But as it turns out, `from` puts the
> original exception on the `__cause__` in *addition* to `__context__`:
>
> [snip]
> So that is not a concern.
>

Awesome! I did not know that.



> > Regarding automatically enforcing this format going forward: I looked
> > at the list of Flake8 rules  and
> > couldn't find anything about it.
> >
>
> Indeed, I don't think there can be -- it cannot be a style violation to
> run into an error while handling another error...
>

I think it can be, as it's very rare to have a legitimate exception without
a "raise from" inside an except clause. In almost any context in which
"during handling of..." is correct, the raising is done deeper in the stack.

I think it's rare enough that personally, I would have liked to have a
Flake8 / PyLint rule like that enforces it, and allow ignoring it with a
comment. (As much as I hate Lint ignore comments.)

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

Re: Use "raise from" where appropriate, all over the codebase

[Shai Berger]

Hi all,

On Sat, 18 Jan 2020 14:27:23 +0200
Ram Rachum  wrote:

> [...] In any case, the
> way Python chains exceptions when showing them is orthogonal to this
> proposed change. Python already displays the exceptions chained even
> if we don't use "raise from", the only thing that "raise from"
> changes is the text that Python puts between the 2 chained exceptions.
> 

Indeed. Well, almost: there is one more thing that addnig the `from`
changes, and that is the attributes on the exception (actually, it's
these attributes which really control the display): Without `from`, the
original exception is placed in a `__context__` attribute; with `from`,
it is placed in a `__cause__` attribute.

At first I thought this was a reason to object to this change -- I
thought code which catches exceptions and looks into their `__context__`
might break because of it. But as it turns out, `from` puts the
original exception on the `__cause__` in *addition* to `__context__`:

In [8]: try:
   ...: try:
   ...: 1/0
   ...: except ZeroDivisionError as e:
   ...: raise Exception("hi") from e
   ...: except Exception as ex:
   ...: exc = ex
   ...: 

In [10]: exc
Out[10]: Exception('hi')

In [13]: exc.__cause__ is exc.__context__
Out[13]: True

So that is not a concern.

> Regarding automatically enforcing this format going forward: I looked
> at the list of Flake8 rules  and
> couldn't find anything about it.
> 

Indeed, I don't think there can be -- it cannot be a style violation to
run into an error while handling another error...

Shai.

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

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

Hi Uri,

All the files were edited manually by me. I used a crude regex to find the
relevant locations: "except .{3,100}raise"

I bet that there are a few cases that my regex didn't cover, but it
probably covered 90%, so we can first decide whether we want this change,
and later worry about the other 10%.

‪On Sat, Jan 18, 2020 at 2:37 PM ‫אורי‬‎  wrote:‬

> Ram,
>
> I noticed that 100 files changed in this commit. Did you edit each file
> manually before you committed, or was it some script doing it for you?
>
> If it was a script or program, can I see it?
>
> Uri.
> אורי
> u...@speedy.net
>
>
> On Sat, Jan 18, 2020 at 11:55 AM Ram Rachum  wrote:
>
>> Hi guys,
>>
>> I recently made a big ticket/PR to Django, and Shai Berger told me I
>> should first talk about it in this mailing list.
>>
>> This is the ticket: https://code.djangoproject.com/ticket/31177 and its
>> PR: https://github.com/django/django/pull/12339
>>
>> It's a generalization of this ticket that I opened and wrote a patch for
>> a few days ago: https://code.djangoproject.com/ticket/31166 It was
>> discussed and merged.
>>
>> Basically, I think that in any place where we catch an exception, and
>> then wrap it with our own exception for whatever reason, we should use the
>> "raise new_exception from old_exception" syntax rather than just "raise
>> new_exception".
>>
>> This means that when Python displays the stacktrace (and when we display
>> it in the debug page,) this will make it have a text of "this exception is
>> the direct result of" instead of "During handling of the above exception,
>> another exception occurred". This is more accurate, because the latter
>> basically means "there was an error in the process of error-handling,"
>> which is absolutely not the case for us, and could mislead a user.
>>
>> Note that we already started transitioning to "raise from", slowly:
>>  - Commit by Mariusz Felisiak:
>> https://github.com/django/django/commit/84dcd1624784c2239f96a97b3151145a85dfbbe3
>>  - Commit by Diederik van der Boor:
>> https://github.com/django/django/commit/25f21bd2376603c8e233a0a0e5a726a0fdfdd33e
>>  - Commit by Thomas Allison:
>> https://github.com/django/django/commit/3e8b7333904f1ab6aa18eeb508659256f3644816
>>
>> What do you think?
>>
>>
>> Thanks,
>> Ram.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/25086cdc-24ab-4f0a-bdb9-9756551ac170%40googlegroups.com
>> 
>> .
>>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/ibEOt3A9c2M/unsubscribe
> .
> To unsubscribe from this group and all its topics, send an email to
> django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CABD5YeHpYTz06M4ywKKmGB8P%3DAhj6Sv3P7K7%3DjB%3DYpCz6wpxLQ%40mail.gmail.com
> 
> .
>

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

Re: Use "raise from" where appropriate, all over the codebase

[אורי]

Ram,

I noticed that 100 files changed in this commit. Did you edit each file
manually before you committed, or was it some script doing it for you?

If it was a script or program, can I see it?

Uri.
אורי
u...@speedy.net


On Sat, Jan 18, 2020 at 11:55 AM Ram Rachum  wrote:

> Hi guys,
>
> I recently made a big ticket/PR to Django, and Shai Berger told me I
> should first talk about it in this mailing list.
>
> This is the ticket: https://code.djangoproject.com/ticket/31177 and its
> PR: https://github.com/django/django/pull/12339
>
> It's a generalization of this ticket that I opened and wrote a patch for a
> few days ago: https://code.djangoproject.com/ticket/31166 It was
> discussed and merged.
>
> Basically, I think that in any place where we catch an exception, and then
> wrap it with our own exception for whatever reason, we should use the
> "raise new_exception from old_exception" syntax rather than just "raise
> new_exception".
>
> This means that when Python displays the stacktrace (and when we display
> it in the debug page,) this will make it have a text of "this exception is
> the direct result of" instead of "During handling of the above exception,
> another exception occurred". This is more accurate, because the latter
> basically means "there was an error in the process of error-handling,"
> which is absolutely not the case for us, and could mislead a user.
>
> Note that we already started transitioning to "raise from", slowly:
>  - Commit by Mariusz Felisiak:
> https://github.com/django/django/commit/84dcd1624784c2239f96a97b3151145a85dfbbe3
>  - Commit by Diederik van der Boor:
> https://github.com/django/django/commit/25f21bd2376603c8e233a0a0e5a726a0fdfdd33e
>  - Commit by Thomas Allison:
> https://github.com/django/django/commit/3e8b7333904f1ab6aa18eeb508659256f3644816
>
> What do you think?
>
>
> Thanks,
> Ram.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/25086cdc-24ab-4f0a-bdb9-9756551ac170%40googlegroups.com
> 
> .
>

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

Re: Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

Hi Tom and Adam,

I do agree that Python's chained exceptions can be confusing. Of course,
when you really need that exception information to troubleshoot something,
it's an absolute godsend. In any case, the way Python chains exceptions
when showing them is orthogonal to this proposed change. Python already
displays the exceptions chained even if we don't use "raise from", the only
thing that "raise from" changes is the text that Python puts between the 2
chained exceptions.

Regarding automatically enforcing this format going forward: I looked at
the list of Flake8 rules  and couldn't find
anything about it.

On Sat, Jan 18, 2020 at 12:57 PM Adam Johnson  wrote:

> Agree with Tom here.
>
> Is there anything we can do to control the way python displays them?
>
> And how would we ensure the format is kept going forwards? Is there a
> flake8 rule/plugin we could activate to enforce it?
>
> On Sat, 18 Jan 2020 at 10:23, Tom Forbes  wrote:
>
>> I agree with this change from a correctness standpoint but I would like
>> to make the point that chained exceptions might be slightly annoying when
>> displayed via console output, as you see the inner exception first and have
>> to scroll up to see the exception you actually have to handle.
>>
>> Tom
>>
>> On 18 Jan 2020, at 09:55, Ram Rachum  wrote:
>>
>> 
>>
>> Hi guys,
>>
>> I recently made a big ticket/PR to Django, and Shai Berger told me I
>> should first talk about it in this mailing list.
>>
>> This is the ticket: https://code.djangoproject.com/ticket/31177 and its
>> PR: https://github.com/django/django/pull/12339
>>
>> It's a generalization of this ticket that I opened and wrote a patch for
>> a few days ago: https://code.djangoproject.com/ticket/31166 It was
>> discussed and merged.
>>
>> Basically, I think that in any place where we catch an exception, and
>> then wrap it with our own exception for whatever reason, we should use the
>> "raise new_exception from old_exception" syntax rather than just "raise
>> new_exception".
>>
>> This means that when Python displays the stacktrace (and when we display
>> it in the debug page,) this will make it have a text of "this exception is
>> the direct result of" instead of "During handling of the above exception,
>> another exception occurred". This is more accurate, because the latter
>> basically means "there was an error in the process of error-handling,"
>> which is absolutely not the case for us, and could mislead a user.
>>
>> Note that we already started transitioning to "raise from", slowly:
>>  - Commit by Mariusz Felisiak:
>> https://github.com/django/django/commit/84dcd1624784c2239f96a97b3151145a85dfbbe3
>>  - Commit by Diederik van der Boor:
>> https://github.com/django/django/commit/25f21bd2376603c8e233a0a0e5a726a0fdfdd33e
>>  - Commit by Thomas Allison:
>> https://github.com/django/django/commit/3e8b7333904f1ab6aa18eeb508659256f3644816
>>
>> What do you think?
>>
>>
>> Thanks,
>> Ram.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/25086cdc-24ab-4f0a-bdb9-9756551ac170%40googlegroups.com
>> 
>> .
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/74BA6A05-D971-47EF-A0B6-4EF1149E83E6%40tomforb.es
>> 
>> .
>>
> --
> Adam
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/django-developers/ibEOt3A9c2M/unsubscribe
> .
> To unsubscribe from this group and all its topics, send an email to
> django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAMyDDM0x0L%2Ba_r_gGL80qL3eqwMS4OV3%3Du-Hc0Wj3N1ruyQfuA%40mail.gmail.com
> 
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  

Re: Use "raise from" where appropriate, all over the codebase

[Adam Johnson]

Agree with Tom here.

Is there anything we can do to control the way python displays them?

And how would we ensure the format is kept going forwards? Is there a
flake8 rule/plugin we could activate to enforce it?

On Sat, 18 Jan 2020 at 10:23, Tom Forbes  wrote:

> I agree with this change from a correctness standpoint but I would like to
> make the point that chained exceptions might be slightly annoying when
> displayed via console output, as you see the inner exception first and have
> to scroll up to see the exception you actually have to handle.
>
> Tom
>
> On 18 Jan 2020, at 09:55, Ram Rachum  wrote:
>
> 
>
> Hi guys,
>
> I recently made a big ticket/PR to Django, and Shai Berger told me I
> should first talk about it in this mailing list.
>
> This is the ticket: https://code.djangoproject.com/ticket/31177 and its
> PR: https://github.com/django/django/pull/12339
>
> It's a generalization of this ticket that I opened and wrote a patch for a
> few days ago: https://code.djangoproject.com/ticket/31166 It was
> discussed and merged.
>
> Basically, I think that in any place where we catch an exception, and then
> wrap it with our own exception for whatever reason, we should use the
> "raise new_exception from old_exception" syntax rather than just "raise
> new_exception".
>
> This means that when Python displays the stacktrace (and when we display
> it in the debug page,) this will make it have a text of "this exception is
> the direct result of" instead of "During handling of the above exception,
> another exception occurred". This is more accurate, because the latter
> basically means "there was an error in the process of error-handling,"
> which is absolutely not the case for us, and could mislead a user.
>
> Note that we already started transitioning to "raise from", slowly:
>  - Commit by Mariusz Felisiak:
> https://github.com/django/django/commit/84dcd1624784c2239f96a97b3151145a85dfbbe3
>  - Commit by Diederik van der Boor:
> https://github.com/django/django/commit/25f21bd2376603c8e233a0a0e5a726a0fdfdd33e
>  - Commit by Thomas Allison:
> https://github.com/django/django/commit/3e8b7333904f1ab6aa18eeb508659256f3644816
>
> What do you think?
>
>
> Thanks,
> Ram.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/25086cdc-24ab-4f0a-bdb9-9756551ac170%40googlegroups.com
> 
> .
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/74BA6A05-D971-47EF-A0B6-4EF1149E83E6%40tomforb.es
> 
> .
>
-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM0x0L%2Ba_r_gGL80qL3eqwMS4OV3%3Du-Hc0Wj3N1ruyQfuA%40mail.gmail.com.

Re: Use "raise from" where appropriate, all over the codebase

[Tom Forbes]

I agree with this change from a correctness standpoint but I would like to make 
the point that chained exceptions might be slightly annoying when displayed via 
console output, as you see the inner exception first and have to scroll up to 
see the exception you actually have to handle.

Tom

> On 18 Jan 2020, at 09:55, Ram Rachum  wrote:
> 
> 
> Hi guys,
> 
> I recently made a big ticket/PR to Django, and Shai Berger told me I should 
> first talk about it in this mailing list.
> 
> This is the ticket: https://code.djangoproject.com/ticket/31177 and its PR: 
> https://github.com/django/django/pull/12339
> 
> It's a generalization of this ticket that I opened and wrote a patch for a 
> few days ago: https://code.djangoproject.com/ticket/31166 It was discussed 
> and merged.
> 
> Basically, I think that in any place where we catch an exception, and then 
> wrap it with our own exception for whatever reason, we should use the "raise 
> new_exception from old_exception" syntax rather than just "raise 
> new_exception".
> 
> This means that when Python displays the stacktrace (and when we display it 
> in the debug page,) this will make it have a text of "this exception is the 
> direct result of" instead of "During handling of the above exception, another 
> exception occurred". This is more accurate, because the latter basically 
> means "there was an error in the process of error-handling," which is 
> absolutely not the case for us, and could mislead a user.
> 
> Note that we already started transitioning to "raise from", slowly:
>  - Commit by Mariusz Felisiak: 
> https://github.com/django/django/commit/84dcd1624784c2239f96a97b3151145a85dfbbe3
>  - Commit by Diederik van der Boor: 
> https://github.com/django/django/commit/25f21bd2376603c8e233a0a0e5a726a0fdfdd33e
>  - Commit by Thomas Allison: 
> https://github.com/django/django/commit/3e8b7333904f1ab6aa18eeb508659256f3644816
> 
> What do you think? 
> 
> 
> Thanks,
> Ram.
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/25086cdc-24ab-4f0a-bdb9-9756551ac170%40googlegroups.com.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/74BA6A05-D971-47EF-A0B6-4EF1149E83E6%40tomforb.es.

Use "raise from" where appropriate, all over the codebase

[Ram Rachum]

Hi guys,

I recently made a big ticket/PR to Django, and Shai Berger told me I should 
first talk about it in this mailing list.

This is the ticket: https://code.djangoproject.com/ticket/31177 and its PR: 
https://github.com/django/django/pull/12339

It's a generalization of this ticket that I opened and wrote a patch for a 
few days ago: https://code.djangoproject.com/ticket/31166 It was discussed 
and merged.

Basically, I think that in any place where we catch an exception, and then 
wrap it with our own exception for whatever reason, we should use the 
"raise new_exception from old_exception" syntax rather than just "raise 
new_exception".

This means that when Python displays the stacktrace (and when we display it 
in the debug page,) this will make it have a text of "this exception is the 
direct result of" instead of "During handling of the above exception, 
another exception occurred". This is more accurate, because the latter 
basically means "there was an error in the process of error-handling," 
which is absolutely not the case for us, and could mislead a user.

Note that we already started transitioning to "raise from", slowly:
 - Commit by Mariusz Felisiak: 
https://github.com/django/django/commit/84dcd1624784c2239f96a97b3151145a85dfbbe3
 - Commit by Diederik van der Boor: 
https://github.com/django/django/commit/25f21bd2376603c8e233a0a0e5a726a0fdfdd33e
 - Commit by Thomas Allison: 
https://github.com/django/django/commit/3e8b7333904f1ab6aa18eeb508659256f3644816

What do you think? 


Thanks,
Ram.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/25086cdc-24ab-4f0a-bdb9-9756551ac170%40googlegroups.com.