Re: [Django] #22920: AppConfig.create swallows informative exceptions during import time

[Django]

#22920: AppConfig.create swallows informative exceptions during import time
-+-
 Reporter:  bendavis78   |Owner:  aaugustin
 Type:  Bug  |   Status:  closed
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  AppConfig app-   | Triage Stage:  Accepted
  loading ImportError|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Aymeric Augustin ):

 In [changeset:"c981d9140d569d489d338741b6a3d878c4e712da"]:
 {{{
 #!CommitTicketReference repository=""
 revision="c981d9140d569d489d338741b6a3d878c4e712da"
 [1.7.x] Fixed #22920 -- Avoid masking some exceptions.

 If loading an application trigger an ImportError, the details of that
 error were lost in some cases. Thanks Ben Davis for the report.

 Backport of b161c01 from master
 }}}

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #22920: AppConfig.create swallows informative exceptions during import time

[Django]

#22920: AppConfig.create swallows informative exceptions during import time
-+-
 Reporter:  bendavis78   |Owner:  aaugustin
 Type:  Bug  |   Status:  closed
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  AppConfig app-   | Triage Stage:  Accepted
  loading ImportError|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by Aymeric Augustin ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"b161c01c48b4c353d7d1a47e9097391a8eb2e047"]:
 {{{
 #!CommitTicketReference repository=""
 revision="b161c01c48b4c353d7d1a47e9097391a8eb2e047"
 Fixed #22920 -- Avoid masking some exceptions.

 If loading an application trigger an ImportError, the details of that
 error were lost in some cases. Thanks Ben Davis for the report.
 }}}

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #22920: AppConfig.create swallows informative exceptions during import time

[Django]

#22920: AppConfig.create swallows informative exceptions during import time
-+-
 Reporter:  bendavis78   |Owner:  aaugustin
 Type:  Bug  |   Status:  assigned
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  AppConfig app-   | Triage Stage:  Accepted
  loading ImportError|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by aaugustin):

 I'm proposing a less invasive fix in
 [https://github.com/django/django/pull/3144 PR 3144].

 I've already rewritten that method entirely once to address issues I don't
 remember very well. I'd rather not restructure the logic drastically just
 to improve error reporting.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #22920: AppConfig.create swallows informative exceptions during import time

[Django]

#22920: AppConfig.create swallows informative exceptions during import time
-+-
 Reporter:  bendavis78   |Owner:  aaugustin
 Type:  Bug  |   Status:  assigned
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  AppConfig app-   | Triage Stage:  Accepted
  loading ImportError|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by aaugustin):

 bendavis78, did you have the opportunity to review my questions above?

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #22920: AppConfig.create swallows informative exceptions during import time

[Django]

#22920: AppConfig.create swallows informative exceptions during import time
-+-
 Reporter:  bendavis78   |Owner:  aaugustin
 Type:  Bug  |   Status:  assigned
Component:  Core (Other) |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  AppConfig app-   | Triage Stage:  Accepted
  loading ImportError|  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by aaugustin):

 * keywords:  AppConfig app loading ImportError => AppConfig app-loading
 ImportError
 * status:  new => assigned
 * severity:  Release blocker => Normal
 * owner:  nobody => aaugustin
 * stage:  Unreviewed => Accepted


Comment:

 I believe the issue is valid and we should address it.

 Can you confirm that the exception masking happens on Python 2 — due to
 the lack of exception chaining — in this section of the code?

 {{{
 except AttributeError:
 # Emulate the error that "from  import "
 # would raise when  exists but not , with
 # more context (Python just says "cannot import name ...").
 raise ImportError(
 "cannot import name '%s' from '%s'" % (cls_name,
 mod_path))
 }}}

 I'm not very proud of that code and I'm not surprised someone came and
 flamed it ;-)

 

 If I read your patch correctly, at some point, you redo an import that you
 know will fail in order to obtain the correct traceback. Two questions:

 1 - Why wasn't it sufficient to use that trick in the piece of code I
 quoted above?
 2 - Isn't it going to be confusing on Python 3, because of the exception
 chaining ?

 I read your patch carefully but I found it hard to validate because it
 rewrites a section that is inherently difficult to test. Thanks for adding
 tests, though, that's very helpful in any case.

 

 I consider this a minor bug because it doesn't affect functionality, only
 error reporting on projets that don't work anyway. Did I miss the reason
 why you marked it as a blocker?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

Re: [Django] #22920: AppConfig.create swallows informative exceptions during import time

[Django]

#22920: AppConfig.create swallows informative exceptions during import time
-+-
 Reporter:  bendavis78   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Core (Other) |  Version:  master
 Severity:  Release blocker  |   Resolution:
 Keywords:  AppConfig app| Triage Stage:
  loading ImportError|  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by bendavis78):

 * needs_better_patch:   => 0
 * has_patch:  0 => 1
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 PR with tests: https://github.com/django/django/pull/2861

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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

[Django] #22920: AppConfig.create swallows informative exceptions during import time

[Django]

#22920: AppConfig.create swallows informative exceptions during import time
---+---
 Reporter: |  Owner:  nobody
  bendavis78   | Status:  new
 Type:  Bug|Version:  master
Component:  Core   |   Keywords:  AppConfig app loading ImportError
  (Other)  |  Has patch:  0
 Severity:  Release|  UI/UX:  0
  blocker  |
 Triage Stage: |
  Unreviewed   |
Easy pickings:  0  |
---+---
 If you have an `INSTALLED_APPS` entry that doesn't use an AppConfig, and
 that module has more than one level, eg "myproject.myapp",
 `AppConfig.create` will swallow any exceptions that occur during the
 import of that module, losing valuable traceback information. The `create`
 method makes some incorrect assumptions about the entry. Mainly it assumes
 that if the entry has more than one part and the import fails, that it
 must be an `AppConfig` class (which is incorrect).

 I've submitted a PR with a fix for this. Basically the new algorithm is as
 follows:

 1. Split the entry into `mod_path` and `ending`. If it's a single-level
 module, `ending` is empty.
 2. Attempt to import `mod_path` as `module`, allowing exceptions to
 propagate (no try/catch)
 3. Check if `ending` is an attribute of `module`.
   - If the attribute doesn't exist OR the attribute exists and is a
 module, import the full entry as `module` (again, allowing exceptions to
 propagate)
   - If the imported module does NOT a `default_app_config` attr, return
 with a default AppConfig instance.
   - Otherwise, import the module path from `default_app_config` entry as
 `module` (again, allowing exceptions to propagate), and assume the ending
 is a class name.
 4. If `ending` ''was'' an attribute of `module` in step 3, we can assume
 it's an AppConfig class.
 5. At this point we have a legitimate module and an object which should be
 a class, and can continue on to validate the AppConfig class.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

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