From 5ace6591cfb49199befc258ccb256a69c455477e Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Thu, 19 Feb 2015 15:37:43 -0800 Subject: Enhance test_assets to expose #1580 This enhances existing tests so that they detect the issue in #1580. Then I'm going to fix the issue in PR #1587. See #1580 --- pyramid/tests/test_config/test_assets.py | 48 ++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/pyramid/tests/test_config/test_assets.py b/pyramid/tests/test_config/test_assets.py index b605a602d..842c73da6 100644 --- a/pyramid/tests/test_config/test_assets.py +++ b/pyramid/tests/test_config/test_assets.py @@ -54,6 +54,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertEqual(source.package, subpackage) self.assertEqual(source.prefix, 'templates/bar.pt') + resource_name = '' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test_override_asset_package_with_package(self): from pyramid.config.assets import PackageAssetSource config = self._makeOne(autocommit=True) @@ -71,6 +77,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertEqual(source.package, subpackage) self.assertEqual(source.prefix, '') + resource_name = 'templates/bar.pt' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test_override_asset_directory_with_directory(self): from pyramid.config.assets import PackageAssetSource config = self._makeOne(autocommit=True) @@ -88,6 +100,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertEqual(source.package, subpackage) self.assertEqual(source.prefix, 'templates/') + resource_name = 'bar.pt' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test_override_asset_directory_with_package(self): from pyramid.config.assets import PackageAssetSource config = self._makeOne(autocommit=True) @@ -105,6 +123,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertEqual(source.package, subpackage) self.assertEqual(source.prefix, '') + resource_name = 'templates/bar.pt' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test_override_asset_package_with_directory(self): from pyramid.config.assets import PackageAssetSource config = self._makeOne(autocommit=True) @@ -122,6 +146,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertEqual(source.package, subpackage) self.assertEqual(source.prefix, 'templates/') + resource_name = 'bar.pt' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test_override_asset_directory_with_absfile(self): from pyramid.exceptions import ConfigurationError config = self._makeOne() @@ -161,6 +191,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertTrue(isinstance(source, FSAssetSource)) self.assertEqual(source.prefix, abspath) + resource_name = '' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test_override_asset_directory_with_absdirectory(self): from pyramid.config.assets import FSAssetSource config = self._makeOne(autocommit=True) @@ -177,6 +213,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertTrue(isinstance(source, FSAssetSource)) self.assertEqual(source.prefix, abspath) + resource_name = 'bar.pt' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test_override_asset_package_with_absdirectory(self): from pyramid.config.assets import FSAssetSource config = self._makeOne(autocommit=True) @@ -193,6 +235,12 @@ class TestAssetsConfiguratorMixin(unittest.TestCase): self.assertTrue(isinstance(source, FSAssetSource)) self.assertEqual(source.prefix, abspath) + resource_name = 'bar.pt' + expected = os.path.join(here, 'pkgs', 'asset', + 'subpackage', 'templates', 'bar.pt') + self.assertEqual(override.source.get_filename(resource_name), + expected) + def test__override_not_yet_registered(self): from pyramid.interfaces import IPackageOverrides package = DummyPackage('package') -- cgit v1.2.3 From e51295bee250a144adee0d31b4c6d0a62ad27770 Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Thu, 19 Feb 2015 13:57:37 -0800 Subject: Fix asset override with package `AssetsConfiguratorMixin.override_asset` does: ```python __import__(override_package) to_package = sys.modules[override_package] override_source = PackageAssetSource(to_package, override_prefix) ``` so it's assuming that the `package` argument to `PackageAssetSource.__init__` takes a module object. But then `PackageAssetSource` had a bunch of methods that did stuff like: - `pkg_resources.resource_exists(self.package, path)` - `pkg_resources.resource_filename(self.package, path)` - `pkg_resources.resource_stream(self.package, path)` and all these `pkg_resources` functions need their `package_or_requirement` argument to be a **string**; not a module - see https://pythonhosted.org/setuptools/pkg_resources.html#basic-resource-access, which says: > the `package_or_requirement argument` may be either a Python package/module > name (e.g. `foo.bar`) or a `Requirement` instance. This causes errors when overriding assets -- e.g.: I am using Kotti and Kotti has this code (https://github.com/Kotti/Kotti/blob/master/kotti/__init__.py#L251): ```python for override in [a.strip() for a in settings['kotti.asset_overrides'].split() if a.strip()]: config.override_asset(to_override='kotti', override_with=override) ``` A Kotti add-on called kotti_navigation does this (https://github.com/Kotti/kotti_navigation/blob/master/kotti_navigation/__init__.py#L12): ```python settings['kotti.asset_overrides'] += ' kotti_navigation:kotti-overrides/' ``` The above code is all legit as far as I can tell and it works fine in pyramid 1.5.2, but it fails with pyramid master with the following: ```pytb File "/Users/marca/python/virtualenvs/kotti_inventorysvc/lib/python2.7/site-packages/pkg_resources.py", line 959, in resource_filename self, resource_name File "/Users/marca/dev/git-repos/pyramid/pyramid/config/assets.py", line 31, in get_resource_filename filename = overrides.get_filename(resource_name) File "/Users/marca/dev/git-repos/pyramid/pyramid/config/assets.py", line 125, in get_filename result = source.get_filename(path) File "/Users/marca/dev/git-repos/pyramid/pyramid/config/assets.py", line 224, in get_filename if pkg_resources.resource_exists(self.package, path): File "/Users/marca/python/virtualenvs/kotti_inventorysvc/lib/python2.7/site-packages/pkg_resources.py", line 948, in resource_exists return get_provider(package_or_requirement).has_resource(resource_name) File "/Users/marca/python/virtualenvs/kotti_inventorysvc/lib/python2.7/site-packages/pkg_resources.py", line 225, in get_provider __import__(moduleOrReq) TypeError: __import__() argument 1 must be string, not module ``` This was a little tricky to resolve because the `override_asset` function wants to pass a module object to `PackageAssetSource.__init__`, but there are a number of tests in `pyramid/tests/test_config/test_assets.py` that assume that it takes a string. So I ended up making it legal to pass either one, so that I don't have to change as much calling code. See https://github.com/Kotti/kotti_navigation/issues/13 --- pyramid/config/assets.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/pyramid/config/assets.py b/pyramid/config/assets.py index 9da092f08..6dabea358 100644 --- a/pyramid/config/assets.py +++ b/pyramid/config/assets.py @@ -214,6 +214,10 @@ class PackageAssetSource(object): """ def __init__(self, package, prefix): self.package = package + if hasattr(package, '__name__'): + self.pkg_name = package.__name__ + else: + self.pkg_name = package self.prefix = prefix def get_path(self, resource_name): @@ -221,33 +225,33 @@ class PackageAssetSource(object): def get_filename(self, resource_name): path = self.get_path(resource_name) - if pkg_resources.resource_exists(self.package, path): - return pkg_resources.resource_filename(self.package, path) + if pkg_resources.resource_exists(self.pkg_name, path): + return pkg_resources.resource_filename(self.pkg_name, path) def get_stream(self, resource_name): path = self.get_path(resource_name) - if pkg_resources.resource_exists(self.package, path): - return pkg_resources.resource_stream(self.package, path) + if pkg_resources.resource_exists(self.pkg_name, path): + return pkg_resources.resource_stream(self.pkg_name, path) def get_string(self, resource_name): path = self.get_path(resource_name) - if pkg_resources.resource_exists(self.package, path): - return pkg_resources.resource_string(self.package, path) + if pkg_resources.resource_exists(self.pkg_name, path): + return pkg_resources.resource_string(self.pkg_name, path) def exists(self, resource_name): path = self.get_path(resource_name) - if pkg_resources.resource_exists(self.package, path): + if pkg_resources.resource_exists(self.pkg_name, path): return True def isdir(self, resource_name): path = self.get_path(resource_name) - if pkg_resources.resource_exists(self.package, path): - return pkg_resources.resource_isdir(self.package, path) + if pkg_resources.resource_exists(self.pkg_name, path): + return pkg_resources.resource_isdir(self.pkg_name, path) def listdir(self, resource_name): path = self.get_path(resource_name) - if pkg_resources.resource_exists(self.package, path): - return pkg_resources.resource_listdir(self.package, path) + if pkg_resources.resource_exists(self.pkg_name, path): + return pkg_resources.resource_listdir(self.pkg_name, path) class FSAssetSource(object): -- cgit v1.2.3