tests; fix csrf tests #138
Conversation
invenio_rest/errors.py
Outdated
| @@ -12,6 +12,7 @@ | |||
| from __future__ import absolute_import, print_function | |||
|
|
|||
| import json | |||
| import typing as t # Required for tests to pass | |||
There was a problem hiding this comment.
why is it not passing without it?
There was a problem hiding this comment.
This is independent of the changes, it looks like it started failing recently with this:
After checking it looks it's caused by HTTPException that has the t.Optional (we are inheriting from them): https://github.com/pallets/werkzeug/blob/main/src/werkzeug/exceptions.py#L76-L77
I couldn't find a better way to fix this, any ideas are welcomed!
There was a problem hiding this comment.
that seems strange to me. i tested without import typing as t with python3.12 and python3.9, both worked. i checked for python3.9 the pip freeze diff. the only difference was that on github two additional packages were installed, mock and requirements-builder
There was a problem hiding this comment.
We can see it in the actions history that it failed, you can check it here: https://github.com/inveniosoftware/invenio-rest/actions/runs/9908547353/job/27374557091
It was also failing locally with the same error.
There was a problem hiding this comment.
yeah i checked that in the github actions. it is strange that it worked on my local installation and it doesn't work on your local machine and on github.
could you still reproduce it on your local machine?
There was a problem hiding this comment.
Yes, same issue locally with python 3.9 and linux
There was a problem hiding this comment.
hm strange. i have managed to reproduce it on another linux machine now. i solved it with
nitpick_ignore = [
("py:class", "t.Optional"),
]
added to docs/conf.py. i am not sure which solution is the better. normally i would go with docs/conf.py because it doesn't add not used imports in a py file.
There was a problem hiding this comment.
Thanks for sharing this! Indeed it's better to do this for the moment.
|
btw the cern copyright notice could be updated too |
Done! 👍 |
No description provided.