Skip to content

Load JSON using importlib.resources - #2351

Open
efokschaner wants to merge 2 commits into
pyupio:masterfrom
efokschaner:fix-2308
Open

Load JSON using importlib.resources#2351
efokschaner wants to merge 2 commits into
pyupio:masterfrom
efokschaner:fix-2308

Conversation

@efokschaner

@efokschaner efokschaner commented Feb 4, 2022

Copy link
Copy Markdown

This is a updated fix for #2308 and supersedes #2309

It uses importlib.resources which means dropping support for python 3.6 and below.

I tested using the following commands:

> python.exe .\setup.py sdist
> python.exe -m pip install .\dist\safety-db-2021.7.17.tar.gz
> python.exe
>>> import safety_db
>>> dir(safety_db)
['INSECURE', 'INSECURE_FULL', '__all__', '__author__', '__builtins__', '__cached__', '__copyright__', '__doc__', '__file__', '__license__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__title__', '__version__', 'json', 'read_text']
>>> len(safety_db.INSECURE)
1787

I also confirmed the un-packaged dir still works as well:

>>> import data
>>> dir(data)
['INSECURE', 'INSECURE_FULL', '__all__', '__author__', '__builtins__', '__cached__', '__copyright__', '__doc__', '__file__', '__license__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__title__', '__version__', 'json', 'read_text']

This is a updated fix for pyupio#2308
and supersedes pyupio#2309

It uses importlib.resources which means dropping
support for python 3.6 and below
@bollwyvl

bollwyvl commented Feb 4, 2022

Copy link
Copy Markdown

Sure, there's that, as well as pkgutil.get_data which has a longer history of being in stdlib, and might not require the 3.7 change.

@efokschaner

Copy link
Copy Markdown
Author

@yeisonvargasf would you happen to be the right person with whom to discuss how to get this fix merged? I'd appreciate your input.

@yeisonvargasf
yeisonvargasf self-requested a review March 23, 2022 01:21
@yeisonvargasf

Copy link
Copy Markdown
Member

Hi @efokschaner! Thanks for this PR, in PyUp we decided to support 3.6+ in our tools. This change requires 3.7, I would merge it as soon as possible if this removes the 3.7 requirement.

Thank you again for your work!

@efokschaner

Copy link
Copy Markdown
Author

Thanks @yeisonvargasf , I'll take a look at the approach that supports 3.6 then!

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

Successfully merging this pull request may close these issues.

3 participants