Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Move from pkg_resources to importlib.resources #12508

Closed
6 tasks
DMRobertson opened this issue Apr 20, 2022 · 1 comment
Closed
6 tasks

Move from pkg_resources to importlib.resources #12508

DMRobertson opened this issue Apr 20, 2022 · 1 comment
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@DMRobertson
Copy link
Contributor

The documentation for pkg_resources declares the package to be deprecated:

Use of pkg_resources is discouraged in favor of importlib.resources, importlib.metadata, and their backports (importlib_resources, importlib_metadata). Please consider using those libraries instead of pkg_resources.

It would be nice (though by no means critically important) to follow this advice. The standalone backport moduleimportlib_resources provides a migration guide.

Grepping the source tree for pkg_resources, we currently use:

  • pkg_resources.resource_filename
  • pkg_resources.resource_stream
  • pkg_resources.parse_version

The first two are covered by the migration guide. The third can be replaced by packaging.version.Version. We already depend on packaging >= 16.1, and AFAICS from its changelog the Version type and its functionality we use have been present since packaging's initial release.

Once this and #12478 is done, we can remove the development dependency on types-setuptools.

Task list:

  • Migrate uses of resource_filename to importlib.resources
  • Migrate uses of resource_stream to importlib.resources
  • Migrate uses of parse_version to packaging.version.Version
  • Remove dev dependency on setuptools and update poetry lockfile (poetry lock --no-update)
  • Run linter script
  • Run tests (in CI for a PR)
@DMRobertson DMRobertson added good first issue Good for newcomers T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches labels Apr 20, 2022
@DMRobertson
Copy link
Contributor Author

Having tried this out (many thanks @vheuken), it wasn't an easy 1-1 replacement.

The sticking point is that we pass template directories to Jinja2, falling back to a default template directory distributed as a package resource.

# Append the default directory at the end of the list so Jinja can fallback on it
# if a template is missing from any custom directory.
search_directories.append(self.default_template_dir)
# TODO: switch to synapse.util.templates.build_jinja_env
loader = jinja2.FileSystemLoader(search_directories)
env = jinja2.Environment(
loader=loader,
autoescape=jinja2.select_autoescape(),
)

But importlib.resources makes it tricky to get hold of that path. (It's concerned with providing access to specific file-like resources, which may not correspond to a filesystem directory).

#12542 (comment) has more details. I think it's best to drop this for now.

@DMRobertson DMRobertson removed good first issue Good for newcomers P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches labels Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant