After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 749532 - Raise ImportError when override exists but another module does not
Raise ImportError when override exists but another module does not
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: general
Git master
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-18 08:03 UTC by Garrett Regier
Modified: 2015-06-21 19:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Explicitly check if an override exists instead of ImportError (1.95 KB, patch)
2015-05-18 09:33 UTC, Garrett Regier
none Details | Review
Explicitly check if an override exists instead of ImportError v2 (1.95 KB, patch)
2015-05-18 14:41 UTC, Garrett Regier
none Details | Review
Explicitly check if an override exists instead of ImportError v3 (1.96 KB, patch)
2015-06-20 06:54 UTC, Garrett Regier
committed Details | Review

Description Garrett Regier 2015-05-18 08:03:33 UTC
If an override depends on another module and it does not exist then the ImportError will be assumed to mean that the override does not exist[1]. Instead it should first check if the override module exists and then let any exceptions be raised while loading the override.

1. https://git.gnome.org/browse/pygobject/tree/gi/overrides/__init__.py#n109
Comment 1 Garrett Regier 2015-05-18 09:33:14 UTC
Created attachment 303513 [details] [review]
Explicitly check if an override exists instead of ImportError

If an override depended on another module and it did not exist then the raised ImportError was consumed and assumed to mean that the override did not exist. This makes it difficult to diagnose issues with overrides.

This uses pkgutil.get_loader() as it is the easier way to determine if a module exists in both Python 2 and 3 and avoid deprecated functions.
Comment 2 Garrett Regier 2015-05-18 14:41:55 UTC
Created attachment 303531 [details] [review]
Explicitly check if an override exists instead of ImportError v2

Use my work email address for the commit.
Comment 3 Simon Feltman 2015-06-17 03:31:21 UTC
Review of attachment 303531 [details] [review]:

::: gi/overrides/__init__.py
@@ +115,3 @@
+            override_loader = get_loader(override_package_name)
+
+        except Exception:

If the try/except is working around a Python bug which raises an AttributeError, shouldn't we be catching that instead of such a broad exception type?
Comment 4 Garrett Regier 2015-06-20 06:54:00 UTC
Created attachment 305736 [details] [review]
Explicitly check if an override exists instead of ImportError v3

(In reply to Simon Feltman from comment #3)
> Review of attachment 303531 [details] [review] [review]:
> 
> ::: gi/overrides/__init__.py
> @@ +115,3 @@
> +            override_loader = get_loader(override_package_name)
> +
> +        except Exception:
> 
> If the try/except is working around a Python bug which raises an
> AttributeError, shouldn't we be catching that instead of such a broad
> exception type?

Fixed
Comment 5 Simon Feltman 2015-06-20 08:00:24 UTC
Review of attachment 305736 [details] [review]:

Looks good, thanks!
Comment 6 Garrett Regier 2015-06-21 19:54:53 UTC
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.