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

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

Closed
ssbarnea opened this issue May 17, 2021 · 17 comments
Closed

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

ssbarnea opened this issue May 17, 2021 · 17 comments
Labels
Milestone

Comments

@ssbarnea
Copy link

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 ssbarnea changed the title v1.3.4 is not compative with py39 v1.3.4 is not compatible with py39 May 17, 2021
@ssbarnea ssbarnea changed the title v1.3.4 is not compatible with py39 v1.3.3 and v1.3.4 is not compatible with py38/py39 May 17, 2021
ssbarnea added a commit to ansible/molecule that referenced this issue May 17, 2021
ssbarnea added a commit to ansible/molecule that referenced this issue May 17, 2021
@funkyfuture
Copy link
Member

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.

@tadeboro
Copy link

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 9844cb1 is a bad backport of #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 '\*'

@funkyfuture
Copy link
Member

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

@tadeboro
Copy link

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.

@ssbarnea
Copy link
Author

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

RUN pip3.7 install black flake8 pre-commit pytest tox PyYAML Sphinx \
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.

@funkyfuture
Copy link
Member

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
Copy link
Member

funkyfuture commented May 18, 2021

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.

@tadeboro
Copy link

@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:

ignore=E203,W503,W605

That change was introduced in e2c5620#diff-ef2cef9f88b4fe09ca3082140e67f5ad34fb65fb6e228f119d3812261ae51449, but I cannot find the reason behind the addition of that ignore entry.

@ssbarnea
Copy link
Author

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.

@funkyfuture
Copy link
Member

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
Copy link
Member

funkyfuture commented May 23, 2021

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.

@ssbarnea
Copy link
Author

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.

@tadeboro
Copy link

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.

gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Jun 11, 2021
Dependency of Ansible Molecule. Builds, tests and installs fine on all
implementations listed in PYTHON_COMPAT.
Adding 1.3.2 in spite of it not being the latest upstream version
because 1.3.3 and 1.3.4 are known to be broken, see
pyeve/cerberus#568

Signed-off-by: Marek Szuba <marecki@gentoo.org>
@funkyfuture funkyfuture added this to the 1.3.5 milestone Oct 24, 2022
@funkyfuture
Copy link
Member

@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
Copy link
Member

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.

@tpwo2
Copy link

tpwo2 commented Jul 26, 2023

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

@funkyfuture
Copy link
Member

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

@funkyfuture funkyfuture closed this as not planned Won't fix, can't repro, duplicate, stale Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants