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

fix(provisional): traversable.read_text error handling #3314

Merged
merged 3 commits into from
Apr 29, 2022

Conversation

mabdinur
Copy link
Contributor

@mabdinur mabdinur commented Apr 28, 2022

traversable.read_text raises a ValueError in python 3.10

Sample Error with hypothesis==6.45.1:

tests/tracer/test_http.py:3: in <module>
    from hypothesis.provisional import urls
<frozen importlib._bootstrap>:1027: in _find_and_load
    ???
<frozen importlib._bootstrap>:1006: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:688: in _load_unlocked
    ???
.riot/venv_py3103_mock_pytest700_pytest-mock_coverage_pytest-cov_opentracing_hypothesis/lib/python3.10/site-packages/_pytest/assertion/rewrite.py:170: in exec_module
    exec(co, module.__dict__)
.riot/venv_py3103_mock_pytest700_pytest-mock_coverage_pytest-cov_opentracing_hypothesis/lib/python3.10/site-packages/hypothesis/provisional.py:37: in <module>
    _tlds = traversable.read_text().splitlines()
../.pyenv/versions/3.10.3/lib/python3.10/importlib/abc.py:378: in read_text
    with self.open(encoding=encoding) as strm:
../.pyenv/versions/3.10.3/lib/python3.10/importlib/_adapters.py:54: in open
    raise ValueError()
E   ValueError

@Zac-HD
Copy link
Member

Zac-HD commented Apr 28, 2022

See #3310 / 7edec74 (cc @The-Compiler); we'll need to handle ValueError in hypothesis/strategies/_internal/datetime.py.

Then create RELEASE.rst and add yourself to the list in AUTHORS.rst, and we'll be good to go 😁

@The-Compiler
Copy link
Contributor

I'm not entirely convinced this is the right fix. You seem to be getting a DegenerateFiles object back, which should not be happening. It works fine for me:

$ python3.10
Python 3.10.4 (main, Mar 23 2022, 23:05:40) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from importlib import resources
>>> traversable = resources.files("hypothesis.vendor") / "tlds-alpha-by-domain.txt"
>>> traversable
PosixPath('/usr/lib/python3.10/site-packages/hypothesis/vendor/tlds-alpha-by-domain.txt')
>>> _tlds = traversable.read_text().splitlines()
>>> _tlds[:5]
['# Version 2019080400, Last Updated Sun Aug  4 07:07:02 2019 UTC', 'AAA', 'AARP', 'ABARTH', 'ABB']

@Zac-HD
Copy link
Member

Zac-HD commented Apr 28, 2022

I'm not entirely convinced this is the right fix. You seem to be getting a DegenerateFiles object back, which should not be happening.

This does seem to be an installation problem, but if falling back to the older API fixes that I'm happy to take the patch and wait as packaging continues to improve.

@The-Compiler
Copy link
Contributor

It might also obscure errors, however - especially if the old API vanishes in Python 3.12+ somewhen.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 28, 2022

It'll certainly become an error sometime (most likely in ~3 years when Hypothesis drops Python 3.8 support), but I'm hoping that we'll have better handling and/or it'll be less common then.

For today, I'd rather have a working fallback than even a helpful error message, and I don't think we can even deliver the latter. Reasonable to leave a comment mentioning DegenerateFiles and pointing to this issue though.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again @mabdinur 💖

@Zac-HD Zac-HD enabled auto-merge April 29, 2022 22:47
@Zac-HD Zac-HD merged commit d7297f4 into HypothesisWorks:master Apr 29, 2022
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.

None yet

3 participants