Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg_resources is deprecated #3731

Open
luhn opened this issue Aug 11, 2023 · 12 comments
Open

pkg_resources is deprecated #3731

luhn opened this issue Aug 11, 2023 · 12 comments

Comments

@luhn
Copy link
Contributor

luhn commented Aug 11, 2023

As of setuptools v67.5.0, a deprecation warning is raised when importing pkg_resources:

DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html

Apparently this has been replaced by importlib.resources and importlib.metadata, which is available since Python 3.7, as well as packaging.

This may be fairly tricky to remove, as src/pyramid/config/assets.py digs pretty deep into the pkg_resources API, namely it registers PackageOverrides with pkg_resources.register_loader_type. However I can't figure out why this call is necessary, I can't find where this loader would be used—Maybe something to do with AssetResolver?

The other issue is that pkg_resources would unzip the whole package into a tmp directory when calling resource_filename, whereas importlib just extracts that single file. pyramid.path.package_path depends on this behavior. Possible solutions:

  • Deprecate pyramid.path.package_path, and by extension pramid.path.caller_path, pyramid.asset.asset_spec_from_abspath, and pyramid.resource.resource_path_from_abspath.
  • Change contract so pyramid.path.package_path and it's dependents don't work with zip files (or returning zip "path")
  • Write an implementation that extracts package to tmp directory, mimicking pkg_resources behaviior.
@mmerickel
Copy link
Member

mmerickel commented Aug 11, 2023

The issue is around config.override_asset which overrides how pkg_resources works on a threadlocal basis such that any pkg_resources (and AssetResolver) invocations find the appropriate packages. This is a pretty core feature of the static views and can impact any templating systems hooked into Pyramid as well like pyramid-jinja2, pyramid-mako, pyramid-chameleon, all of which use pkg_resources and are dependent on the asset override features in Pyramid to find the right assets. When I have looked, importlib.resources does not have hooks to implement this functionality and is going to be a sticking point in what Pyramid can do about this issue without requiring setuptools.

I'm not really aware of places that importlib.metadata would be helpful in pyramid but those have been very easy to replace in any package where I've done it to drop setuptools for that purpose.

@luhn
Copy link
Contributor Author

luhn commented Aug 11, 2023

It may be possible to override importlib machinery? A custom Loader can provide a get_resource_reader method, which then can return a ResourceReader object. https://docs.python.org/3.8/library/importlib.html#setting-up-an-importer

Do we want to monkeypatch the import machinery, though? It seems to go against Pyramid's MO of everything contained within a registry and discouraging threadlocals. Now's the time to break from that, as all projects will have to rewrite their pkg_resources usage anyways.

As a counterpoint, however, maintainers will by default follow the migration guide and expect it to just work.

@mmerickel
Copy link
Member

mmerickel commented Aug 12, 2023

I’m gonna be honest I don’t really know what to do about this. It’s been in the back of my mind for a long time and just haven’t seen a path forward.

  • Pyramid could offer its own apis that support the override and we remove the pkg_resources support. Release a patch version that deprecated things so folks can get the warnings.

  • Pyramid could completely remove asset override support.

  • Pyramid could try to patch the loaders and support folks overriding importlib.resources. Keeping the current pkg_resources support but making it a soft dependency instead of hard.

Am I missing an option?

@luhn
Copy link
Contributor Author

luhn commented Aug 12, 2023

As a compromise between one and two, we could keep override support for static views but drop support for global overrides. Static overrides I think are necessary, as you often have static assets stored outside of your Python package, but I'm not convinced how useful it is outside of that.. Perhaps even actively harmful: Can you imagine trying to debug why your app is loading a resource that is not the one in the package, all because of a single override_asset buried somewhere in the codebase?

To ease the transition, we could have a pkg_resources shim that uses importlib.resource by default, but if override_asset is called it the shim switches to pkg_resources. That way any apps relying on overrides will still work for now, while other apps can avoid the deprecation warnings and be unaffected when pkg_resources is eventually removed. A new override_static_asset method (or perhaps an override kwarg for add_static_view?) would provide static-only overrides with no monkeypatching of the import machinery.

@mmerickel
Copy link
Member

The override feature is very useful because it allows some libraries to easily provide tweaks. For example pypi itself used it (not sure if still) to open source most of their ui while keeping parts of it closed because they were owned by the graphic designer. The override allowed a simple theme concept for them.

Also as I said before it’s expected to work for all of the templating libraries for a similar reason.

I guess my point is that it’s definitely not there just for static views.

@mmerickel
Copy link
Member

I'm genuinely at a loss how to deal with this issue. I want to do something to make pkg_resources optional but it's so integral to a some core apis that currently expect a threadlocal place to do a lookup.

I think we'd have to make the asset resolution apis work "correctly" without pkg_resources, and then offer an extra api to opt-in to binding the data to pkg_resources if someone wants it.

Working correctly is the challenge there - basically it means we'd do some form of asset lookup / registry ourselves that can obey config.override_asset to preserve that feature via get_current_registry() instead of going straight to pkg_resources.

@mmerickel
Copy link
Member

We'd then update other libraries like pyramid_jinja2 and pyramid_mako to use whatever API instead. Right now pyramid_jinja2 uses abspath_from_asset_spec which isn't even a public pyramid API right now.

Of course one option is to just make things like abspath_from_asset_spec work as expected without using pkg_resources by starting its lookup at some get_current_registry().queryUtility(IAssetRegistry) that controls the lookup via importlib.resources, pkg_resources, etc.

@mmerickel
Copy link
Member

mmerickel commented Feb 5, 2024

I have removed the easy spots in #3748 and #3749. Down to 4 locations that are impacted:

  • pyramid.assets.abspath_from_asset_spec
  • config.override_asset installs the pkg_resources.DefaultProvider hooks
  • pyramid.path.PkgResourcesAssetDescriptor
  • pyramid.static uses direct apis that should probably all be replaced with pyramid.path.AssetResolver.

Ultimately the focus is:

  • Make pyramid.path.AssetResolver work with a new AssetSpecDescriptor implementing IAssetDescriptor that can "do the right thing" by respecting config.override_asset().
  • Fix everything within pyramid to use pyramid.path.AssetResolver.
    • pyramid.asset.abspath_from_asset_spec
    • pyramid.static
  • Create something like config.install_pkg_resources_asset_provider()

@mmerickel
Copy link
Member

Related, I opened this conversation with importlib_resources after diving into this a bit.

python/importlib_resources#295

@mmerickel
Copy link
Member

mmerickel commented Feb 8, 2024

This isn't a blocker, it'd just be nice if Pyramid could support propagating asset overrides into importlib.resources.

Pyramid should offer its own apis (it does, AssetResolver), that work properly without those dependencies, and that's the focus here - try to get Pyramid to work internally without pkg_resources doing all the work for it. That means a new impl will need to use importlib.resources APIs to actually load assets. But it needs to do work before that to determine which asset to load based on asset overrides. Previously we were pushing all that work into pkg_resources to do for us via the IPackageOverrides object. All of that needs to be moved into AssetResolver instead.

@robvdl
Copy link
Contributor

robvdl commented Mar 4, 2024

Just an observation, but the project seems to be in a transitional phase and stuff is spread out across pyproject.toml, setup.py and setup.cfg which is quite confusing. But Pyramid being an older project that totally makes sense.

My guess is that eventually the setup.py and setup.cfg will be dropped.

@mmerickel
Copy link
Member

It's true we will at some point port most of our project metadata into pyproject.toml but this ticket isn't about how we're using setuptools to build Pyramid. It's specifically about using pkg_resources within the actual pyramid package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants