cerberus icon indicating copy to clipboard operation
cerberus copied to clipboard

v1.3.3 and v1.3.4 is not compatible with py38/py39

Open ssbarnea opened this issue 3 years ago • 13 comments

Despite the fact that Cerberus==1.3.4 list compatibility with py39 we can easily get:

.tox/py39/lib/python3.9/site-packages/cerberus/validator.py:19: in <module>
    from cerberus import errors
E     File "/Users/ssbarnea/c/a/molecule/.tox/py39/lib/python3.9/site-packages/cerberus/errors.py", line 156
E       """
E       ^
E   SyntaxError: invalid escape sequence \*

While this bug was fixed in master it is not of any help for consumers of the library as there is no newer release. Even a pre-release like 2.0.0a0 could help as it would allow us to request it when running on py39 in setup.cfg.

Another approach would be backporting the fixed and making another hotfix release.

ssbarnea avatar May 17 '21 14:05 ssbarnea

the error message isn't telling me anything. might it be related to an optimization level of the interpreter?

because this runs fine and i have an application here that runs on 3.9 with Cerberus 1.3.4.

funkyfuture avatar May 17 '21 17:05 funkyfuture

I am not sure if this has anything to do with Python 3.9. The docstring that the error points to contains an invalid escape sequence (\*) and that error is not related to python 3.9 at all.

It looks like https://github.com/pyeve/cerberus/commit/9844cb18ba62952e630cf2151b5bbceffe4b6c4e is a bad backport of https://github.com/pyeve/cerberus/pull/538 (the original PR uses r""" ... """ while the backport does not add the r prefix).

This is what flake8 has to say about this:

$ flake8 venv/lib/python3.9/site-packages/cerberus/errors.py
venv/lib/python3.9/site-packages/cerberus/errors.py:157: [W605] invalid escape sequence '\*'
venv/lib/python3.9/site-packages/cerberus/errors.py:185: [W605] invalid escape sequence '\*'

tadeboro avatar May 17 '21 20:05 tadeboro

yes, this seems more likely to be the cause. but i'm still clueless under which circumstances the \* in docstings causes errors.

funkyfuture avatar May 18 '21 08:05 funkyfuture

I cannot replicate the error on my Linux machines, but it looks like the original error was produced on a mac, so maybe that makes the difference? But in any case, the Cerberus has a syntax error in it that should probably be fixed.

tadeboro avatar May 18 '21 09:05 tadeboro

I should mention that I did not call flake8, that was a runtime error with molecule I seen on mac. Any of the two newer releases gave this error but the previous one worked.

I know that newer versions of python became more picky about invalid escapes in strings but I am not sure why this was not identified on all platforms.

I would advise upgrading https://github.com/pyeve/cerberus/blob/61335ca50242a3cb5ee577cd96769993c050c4bb/Dockerfile#L6 to use latest version of python supported by the project. Over the years I observed that flake8 finds far more problems when run with more modern python versions.

ssbarnea avatar May 18 '21 11:05 ssbarnea

the Dockerfile isn't used for the CI tests, it's obsolete. could you please open a PR that extends .github/workflows/qa.yml with tests on MacOS against the 1.3.x branch? if that can reproduce your error, a subsequent fix would help as well. obviously. my time is very limited atm.

funkyfuture avatar May 18 '21 13:05 funkyfuture

i can't reproduce a failure when the unit tests run on MacOS w/ GH Actions.

also, i don't get the errors with flake8 3.9.1 (mccabe: 0.6.1, pycodestyle: 2.7.0, pyflakes: 2.3.1) CPython 3.9.4 on Linux that @tadeboro posted.

what's the exact interpreter version you're using? which MacOS is it?

besides, could you verify that prefixing the docstrings as raw strings avoids the failure?

@ssbarnea, a git bisect might be helpful.

funkyfuture avatar May 18 '21 20:05 funkyfuture

@funkyfuture Are you running flake8 from the repo? The invalid escape sequence will not be reported there because the tox.ini file instructs flake8 to ignore that error: https://github.com/pyeve/cerberus/blob/a70b404e5bf164bb6eaf1d7f264957db4ab7d55c/tox.ini#L28

That change was introduced in https://github.com/pyeve/cerberus/commit/e2c5620ad88e8e3f20edadb66d2c60793b44d338#diff-ef2cef9f88b4fe09ca3082140e67f5ad34fb65fb6e228f119d3812261ae51449, but I cannot find the reason behind the addition of that ignore entry.

tadeboro avatar May 18 '21 21:05 tadeboro

That explains why it was not identified by flake8. I am using python 3.9.2 installed using pyenv, but now we should be able to address it.

ssbarnea avatar May 19 '21 08:05 ssbarnea

i'm not really convinced by addressing it with flake8 only. i'd like to see a crashing interpreter. what is the MacOS release you encountered this?

I cannot find the reason behind the addition of that ignore entry.

unfortunately i can't remember.

funkyfuture avatar May 19 '21 19:05 funkyfuture

i tried to reproduce the behaviour on a MacOS Big Sur without success:

$ brew install pyenv
$ pyenv install 3.9.4
$ brew install pipx
$ pipx install pew
$ cd ~/cerberus
$ git switch 1.3.x
$ pew mktmpenv -p ~/.pyenv/versions/3.9.4/bin/python
$ pip install pytest
$ pytest cerberus/tests
…
platform darwin -- Python 3.9.4, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
…
241 passed, 1 skipped, 19 warnings in 1.70s

i used a virtual environment here, because i didn't get the --discovery option of tox working.

funkyfuture avatar May 23 '21 12:05 funkyfuture

I suspect that calling python interpreter with a more strict mode may trigger this. See https://docs.python.org/3/library/warnings.html

The escape is still wrong but I think that the warning is filtered by default, unless someone runs with -W or similar options, something the some projects do when running their own test suites.

ssbarnea avatar May 23 '21 15:05 ssbarnea

The invalid escape sequence warning is silenced by default because it would annoy people maintaining ancient codebases written when those escape sequences were valid. Python interpreted them as literal backslash plus whatever comes after it. But "THe Right Way (TM)" that Python devs suggest here is to either use raw string literals or escape the backslash.

tadeboro avatar May 23 '21 16:05 tadeboro

@ssbarnea i intend to release a 1.3.5 release within the next weeks. i'd appreciate if you would try to reproduce the bug with what's available in the 1.3.x branch now and report back the result.

funkyfuture avatar Jul 23 '23 13:07 funkyfuture

i tried to replicate that again as previously but against Python 3.11.4 and neither does it crash nor am i seeing any of the warnings.

funkyfuture avatar Jul 26 '23 14:07 funkyfuture

We're still using Python 3.8 at work, and a long time ago we downgraded to Cerberus 1.3.2 due to this issue. Tomorrow I can check if 1.3.4 works for us.

EDIT: now I actually read the whole thread. A previous developer, who already switched job, bound Cerberus to 1.3.2 with comment to this issue, so we thought 1.3.4 is much more problematic...

I cannot replicate the error on my Linux machines, but it looks like the original error was produced on a mac, so maybe that makes the difference?

We're also running the code on Linux machines and 1.3.4 works without any issues

tpwo2 avatar Jul 26 '23 14:07 tpwo2

i assume that the described error can't be reproduced.

funkyfuture avatar Aug 04 '23 09:08 funkyfuture