Skip to content

Load schemas via importlib.resources instead of pkgutil #871

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

Closed
sevein opened this issue Nov 2, 2021 · 3 comments · Fixed by #873
Closed

Load schemas via importlib.resources instead of pkgutil #871

sevein opened this issue Nov 2, 2021 · 3 comments · Fixed by #873
Labels
Enhancement Some new desired functionality Help Wanted An enhancement or bug for which a pull request is welcomed and which should have a clear definition.

Comments

@sevein
Copy link
Contributor

sevein commented Nov 2, 2021

Producing binaries that embed Python applications that import jsonschema is challenging because pkgutil.get_data relies on __file__ which may not be always defined, e.g. see these issues in PyOxidizer.

https://github.com/Julian/jsonschema/blob/272b4f24437d9cba1127058dc3479f9bc6d8c4b9/jsonschema/_utils.py#L49-L55

I was wondering if this project would be interested in having a fallback implemented that is capable to load the schema files using importlib.resources instead, which was added in Python 3.7. See https://stackoverflow.com/a/20885799 for more details. The creator of PyOxidizer, Gregory Szorc, shares some good details on loading resource files using PyOxidizer here.

@Julian
Copy link
Member

Julian commented Nov 3, 2021

Hi. importlib.resources is going through some instability now (having lots of APIs deprecated), which is something I haven't paid a ton of attention to.

In theory now that we're 3.7+ I'm indeed open to the idea of using it if we can do so without raising deprecation warnings on other versions, so a PR would be welcome --

I'll also just point out though that support for pyoxidizer (or pyinstaller, or cx_freeze, or etc.) is unofficial because jsonschema isn't run under it in CI, so whilst I'm OK with this proposal for independent reasons (that importlib.resources is the new way to do this), I can't really promise in the future that things will continue to work.

@Julian Julian added Enhancement Some new desired functionality Help Wanted An enhancement or bug for which a pull request is welcomed and which should have a clear definition. labels Nov 3, 2021
@Julian Julian changed the title load_schema fallback when pkgutil.get_data doesn't work Load schemas via importlib.resources instead of pkgutil Nov 3, 2021
@sevein
Copy link
Contributor Author

sevein commented Nov 3, 2021

Thank you, @Julian. I woud love to submit a pull request!

If we want to use importlib.resources.read_text then I believe we need to convert schemas into a sub-package (adding an empty __init__.py file). I think this is one of the functions that may be eventually deprecated but available in the standard library since Python 3.7.

The alternative seems to be the much nicer importlib.resources.files, added in Python 3.9 but also in older releases of Python if we used importlib_resources.

Let me know what's your preference. Thanks again.

@Julian
Copy link
Member

Julian commented Nov 3, 2021

Cool!

I'd rather not use a function that's already been deprecated, unless it shakes out that it's going to be undeprecated which last I saw seemed unclear that that'd happen.

So I'd prefer the second route with the backport for pre-3.9, which I hear has its own perils but seems at least like the supported route by the upstream folks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Some new desired functionality Help Wanted An enhancement or bug for which a pull request is welcomed and which should have a clear definition.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants