Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+-
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  closed
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:  fixed
 Keywords: | Triage Stage:  Ready for checkin
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+-

Comment (by Tim Graham ):

 In [changeset:"d029fafea1a1f2f257397fbcd0fa3b531e09854f"]:
 {{{
 #!CommitTicketReference repository=""
 revision="d029fafea1a1f2f257397fbcd0fa3b531e09854f"
 Removed utils.module_loading.import_by_path() per deprecation timeline;
 refs #21674.
 }}}

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

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+-
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  closed
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:  fixed
 Keywords: | Triage Stage:  Ready for checkin
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+-
Changes (by Tim Graham ):

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


Comment:

 In [changeset:"5d263dee304fdaf95e18d2f0619d6925984a7f02"]:
 {{{
 #!CommitTicketReference repository=""
 revision="5d263dee304fdaf95e18d2f0619d6925984a7f02"
 Fixed #21674 -- Deprecated the import_by_path() function in favor of
 import_string().

 Thanks Aymeric Augustin for the suggestion and review.
 }}}

-- 
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/067.1968ed375861bd480315126b0922d7ca%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+-
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Ready for checkin
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+-

Comment (by aaugustin):

 That would be acceptable.

 The biggest problem is that Django raises and catches ImproperlyConfigured
 too liberally. As long as we avoid that, I'm happy.

-- 
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/067.65bfb9cce70929601fedf31a79032686%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+-
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Ready for checkin
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+-

Comment (by timo):

 My only reservation is that I think it's a bit confusing to have to catch
 `AttributeError` and `ValueError` (besides `ImportError`) for all usage of
 `import_string`. Do you think catching these exceptions inside
 `import_string` and re-raising `ImportError` for these cases would be
 considered harmful?

-- 
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/067.ec1b26a898b43218d6e03182a0b7fc7b%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+-
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Ready for checkin
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+-
Changes (by aaugustin):

 * stage:  Accepted => Ready for checkin


Comment:

 The PR has been heavily reviewed by Timo and improved by Berker. I think
 it's ready. Thank you very much.

 Timo, I'll let you do the merge, in case you want to review Berker's
 latest changes first.

-- 
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/067.f4dfff7f9442061256ebb319412e7fa1%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+
Changes (by berkerpeksag):

 * needs_better_patch:  1 => 0


Comment:

 I've updated my pull request to adress Aymeric and Tim's feedbacks:
 https://github.com/django/django/pull/2187

 Thanks!

-- 
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/067.247a2e50fdda5d1369b4fd62a43937fe%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  1
Easy pickings:  0  |UI/UX:  0
---+
Changes (by timo):

 * needs_better_patch:  0 => 1


Comment:

 Patch needs improvement as noted by Aymeric. Usage of the deprecated
 function also needs to be removed as I noted on the PR.

-- 
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/067.2eaae6710832fa3fd7f5fd2089a1b322%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+

Comment (by aaugustin):

 Really, I'm not eager to add an error masking API such as `error_class`.
 What's wrong with a plain ImportError? Usually it's pretty clear.

 If a _caller_ can provide additional information for its specific use-
 case, it can catch and re-raise an exception. Exception chaining makes
 this less of a problem on Python 3. As long as we support Python 2, I'm -1
 on catching an exception and raising another one.

 I would suggest adding a new function that simply perfoms the import and
 doesn't handle exceptions. We would use it wherever the `error_prefix`
 argument of `import_by_path` isn't needed. We would make this function
 public and recommend it rather than `import_by_path`. Regarding its name,
 `import_string` sounds reasonable.

-- 
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/067.c51531d48b6807695b003639d43c3cfe%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+

Comment (by berkerpeksag):

 Ah, good idea. Thanks claudep.

 How about adding a new keyword argument named `suppress_import_error`?
 (somewhat similar to
 http://docs.python.org/3.4/library/contextlib.html#contextlib.suppress)

 {{{#!python
 def import_by_path(dotted_path, error_prefix='',
 suppress_import_error=True):  # or raise_import_error=False
 # snip
 except ImportError as e:
 if not suppress_import_error:
 raise
 raise ImproperlyConfigured
 # snip
 }}}

-- 
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/067.7592739ae2b60c42d8b081384c0ee0b1%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+

Comment (by claudep):

 I think that the `ImproperlyConfigured` exception might make sense in some
 cases, especially when dotted_path is coming from a setting. What about
 adding a new keyword argument to `import_by_path`, something like: `def
 import_by_path(dotted_path, error_prefix='',
 error_class=ImproperlyConfigured)`. Then it would be possible to overwrite
 the exception raised by that function, on a case-by-case basis.

-- 
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/067.922f81235892b970b0be7ea82f117bc7%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+
 Reporter:  aaugustin  |Owner:  berkerpeksag
 Type:  Bug|   Status:  assigned
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+
Changes (by berkerpeksag):

 * cc: berker.peksag@… (added)
 * owner:  nobody => berkerpeksag
 * has_patch:  0 => 1
 * status:  new => assigned


Comment:

 I've opened a pull request on GitHub:
 https://github.com/django/django/pull/2187

 I borrowed the name "import_string" from the Peak project:

 http://svn.eby-sarna.com/Importing/peak/util/imports.py?view=markup

-- 
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/067.baf87e6d389f43846f4145fd1159e322%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+
 Reporter:  aaugustin  |Owner:  nobody
 Type:  Bug|   Status:  new
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+
Changes (by timo):

 * stage:  Unreviewed => Accepted


-- 
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/067.cd787739cfd9b762074162a48782b4cf%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+--
 Reporter:  aaugustin  |Owner:  nobody
 Type:  Bug|   Status:  new
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by aaugustin):

 Hum, no, it's documented. Well we could provide a replacement in Django
 and then deprecate it. It was only added in 1.6.

-- 
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/067.a735131f6b2fc2832f4bafd567c17a18%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+--
 Reporter:  aaugustin  |Owner:  nobody
 Type:  Bug|   Status:  new
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by aaugustin):

 Well, it's a private API...

-- 
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/067.a16ffae1a78a53b0c01b1e2e56be6dac%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
---+--
 Reporter:  aaugustin  |Owner:  nobody
 Type:  Bug|   Status:  new
Component:  Utilities  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by timo):

 Do you see any way we can make the change backwards compatible?

-- 
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/067.93dbe5bbcf561353f6636a51aa398d24%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Django] #21674: django.utils.module_loading.import_by_path considered harmful

[Django]

#21674: django.utils.module_loading.import_by_path considered harmful
--+
   Reporter:  aaugustin   |  Owner:  nobody
   Type:  Bug | Status:  new
  Component:  Utilities   |Version:  master
   Severity:  Normal  |   Keywords:
   Triage Stage:  Unreviewed  |  Has patch:  0
Needs documentation:  0   |Needs tests:  0
Patch needs improvement:  0   |  Easy pickings:  0
  UI/UX:  0   |
--+
 The purpose of this function is to import whatever its argument points to.

 If it fails, it should raise ImportError, signalling that an import
 failed.

 Unfortunately, it catches exceptions, including ImportErrors, and re-
 raises ImproperlyConfigured. Such exception masking makes it needlessly
 hard to diagnose circular import problems, because it makes it look like
 the problem comes from inside Django. It becomes supremely perverse when
 some code in Django catches ImproperlyConfigured and things go wrong
 further down the line.

 I understand that the original intent was to provide more frienly error
 messages, but I believe that ImportError is a perfectly fine and suitable
 exception and that replacing it with a more generic one is a net loss.

 (I know I'm attacking an old dogma, but this is an easy step in the long
 standing "improved error reporting" project.)

-- 
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/052.262ca8e54da3a22009b272d2b7999e7b%40djangoproject.com.
For more options, visit https://groups.google.com/groups/opt_out.