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

browsers.json not loading on Python 3.7, 3.8 & 3.9 #153

Closed
jayvdb opened this issue Nov 19, 2022 · 11 comments · Fixed by #154
Closed

browsers.json not loading on Python 3.7, 3.8 & 3.9 #153

jayvdb opened this issue Nov 19, 2022 · 11 comments · Fixed by #154

Comments

@jayvdb
Copy link

jayvdb commented Nov 19, 2022

Following on from #139 , there are still tests failing when there is no internet, but not failing on Python 3.10.

On Python 3.9, without internet, the failures are

[   39s] FAILED tests/test_fake.py::TestFake::test_fake_init - fake_useragent.errors.F...
[   39s] FAILED tests/test_fake.py::TestFake::test_fake_safe_attrs - fake_useragent.er...
[   39s] FAILED tests/test_fake.py::TestFake::test_fake_user_agent_browsers - fake_use...
[   39s] FAILED tests/test_utils.py::TestUtils::test_utils_load - fake_useragent.error...
[   39s] FAILED tests/test_utils.py::TestUtils::test_utils_load_use_local_file - fake_...

In all cases, the log includes

[   39s] WARNING  fake_useragent:utils.py:127 Could not find local data/json file or could not parse the contents. Fallback to external resource.
[   39s] Traceback (most recent call last):
[   39s]   File "/home/abuild/rpmbuild/BUILDROOT/python-fake-useragent-1.0.0-16.1.x86_64/usr/lib/python3.9/site-packages/fake_useragent/utils.py", line 120, in load
[   39s]     ilr.files("fake_useragent.data").joinpath("browsers.json").read_text()
[   39s]   File "/usr/lib64/python3.9/importlib/resources.py", line 147, in files
[   39s]     return _common.from_package(_get_package(package))
[   39s]   File "/usr/lib64/python3.9/importlib/_common.py", line 14, in from_package
[   39s]     return fallback_resources(package.__spec__)
[   39s]   File "/usr/lib64/python3.9/importlib/_common.py", line 18, in fallback_resources
[   39s]     package_directory = pathlib.Path(spec.origin).parent
[   39s]   File "/usr/lib64/python3.9/pathlib.py", line 1082, in __new__
[   39s]     self = cls._from_parts(args, init=False)
[   39s]   File "/usr/lib64/python3.9/pathlib.py", line 707, in _from_parts
[   39s]     drv, root, parts = self._parse_args(args)
[   39s]   File "/usr/lib64/python3.9/pathlib.py", line 691, in _parse_args
[   39s]     a = os.fspath(a)
[   39s] TypeError: expected str, bytes or os.PathLike object, not NoneType

Note these errors may not be seen in CI because they will fallback to fetching the data.

Note browsers.json is installed correctly.

One logic problem is
https://github.com/fake-useragent/fake-useragent/blob/master/src/fake_useragent/utils.py#L10

As Python 3.7 have https://docs.python.org/3/library/importlib.resources.html , they wont try to import importlib_resources.

The import needs to try to import files to see if it exists. That will fix Python 3.7 & 3.8, but doesnt explain why Python 3.9 is failing. It could be that Python 3.10 .files() works a bit better than Python 3.9.

Note you can use https://pypi.org/project/pytest-socket/ to simulate no Internet.

@melroy89
Copy link
Collaborator

melroy89 commented Nov 19, 2022

Does python3.7, 3.8, 3.9 all have importlib_resources.files? Since I can try to invert import of the packages.. So trying to import import importlib_resources as ilr first.

If that might fix the issue using the backports.

ps. Good catch btw!

@melroy89
Copy link
Collaborator

Like this: #154

@melroy89
Copy link
Collaborator

But would this solve the problem? For some reason it's very hard to setup multiple different (virtual) python version environments locally.

I could try to use docker I guess.

@jayvdb
Copy link
Author

jayvdb commented Nov 19, 2022

https://github.com/nektos/act is useful for running workflows locally, but note I found it problematic on Windows.

@jayvdb
Copy link
Author

jayvdb commented Nov 19, 2022

Yes, forcing using https://pypi.org/project/importlib-resources/ (https://github.com/python/importlib_resources) will work, as that is always more correct than the CPython stdlib version.

@melroy89
Copy link
Collaborator

Or here it is stated as well

importlib.resources was added to Python 3.7. However, the API illustrated in this code (using files()) was added only in Python 3.9, [3] and support for accessing data files via namespace packages was added only in Python 3.10 [4] (the data subdirectory is a namespace package under the root package mypkg). Therefore, you may find this code to work only in Python 3.10 (and above). For other versions of Python, you are recommended to use the importlib-resources backport which provides the latest version of this library. In this case, the only change that has to be made to the above code is to replace importlib.resources with importlib_resources, i.e.

https://setuptools.pypa.io/en/latest/userguide/datafiles.html#accessing-data-files-at-runtime

@melroy89 melroy89 reopened this Nov 19, 2022
@melroy89
Copy link
Collaborator

melroy89 commented Nov 19, 2022

I tried to fix it in v1.0.1 release. You can maybe validate it as well on our setup with the different Python releases?

@jayvdb
Copy link
Author

jayvdb commented Nov 20, 2022

Thanks, this is now fixed.
https://build.opensuse.org/request/show/1036859 is a nice clean & simple spec file now.
It is a bit unfortunate that the PyPI tarball no longer includes the tests (because MANIFEST.in was removed), as PyPI is the preferred source for Python packages. Getting the tarballs from GitHub is a bit less automated.

@jayvdb jayvdb closed this as completed Nov 20, 2022
@melroy89
Copy link
Collaborator

Ah right.. regarding the missing testcases, maybe I can just add that in pyproject.toml instead? I think this MANIFEST.in was redundant / unnecessary in several ways by now.

Like so?

packages = [
    { include = "tests"},
]

@jayvdb
Copy link
Author

jayvdb commented Nov 20, 2022

Ya, there are newer ways of doing it now with various build backends.

@melroy89
Copy link
Collaborator

Ya, there are newer ways of doing it now with various build backends.

Indeed.

"The Zen of Python"

There should be one-- and preferably only one --obvious way to do it.

Well, that is not really the case anymore.. haha

Try:

import this

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 a pull request may close this issue.

2 participants