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

incompatible with werkzeug 2.2.0, can't import parse_rule #686

Closed
frantisekz opened this issue Jul 11, 2022 · 11 comments · Fixed by #691
Closed

incompatible with werkzeug 2.2.0, can't import parse_rule #686

frantisekz opened this issue Jul 11, 2022 · 11 comments · Fixed by #691
Assignees

Comments

@frantisekz
Copy link

Importing flask-login with werkzeug 2.2.0a1 ends up with:

Traceback:
/usr/lib64/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_packager_dashboard_pagure.py:4: in <module>
    from oraculum.utils import pagure
oraculum/__init__.py:29: in <module>
    from flask_login import LoginManager
/usr/lib/python3.11/site-packages/flask_login/__init__.py:12: in <module>
    from .login_manager import LoginManager
/usr/lib/python3.11/site-packages/flask_login/login_manager.py:35: in <module>
    from .utils import _create_identifier
/usr/lib/python3.11/site-packages/flask_login/utils.py:14: in <module>
    from werkzeug.routing import parse_rule
E   ImportError: cannot import name 'parse_rule' from 'werkzeug.routing' (/usr/lib/python3.11/site-packages/werkzeug/routing/__init__.py)
@davidism
Copy link
Collaborator

davidism commented Jul 11, 2022

I'm trying to figure out what that code does in the first place. I think it's trying to say "if there's a dynamic subdomain or host matching in the current rule, apply that same argument to the login rule." But there's surely a better way to go about that.

if login_view.startswith(("https://", "http://", "/")):
return login_view
else:
try:
url_rule = request.url_rule.subdomain or request.url_rule.host
except AttributeError:
url_rule = None
if request.view_args and url_rule:
args = {}
for _, _, key in parse_rule(url_rule):
if not key or key not in request.view_args:
continue
args[key] = request.view_args[key]
return url_for(login_view, **args)
else:
return url_for(login_view)

@najibfahs

This comment was marked as off-topic.

@potiuk

This comment was marked as off-topic.

Zlopez added a commit to Zlopez/anitya that referenced this issue Jul 25, 2022
Limit werkzeug version till
maxcountryman/flask-login#686 is fixed.

Signed-off-by: Michal Konečný <[email protected]>
Zlopez added a commit to fedora-infra/anitya that referenced this issue Jul 25, 2022
Limit werkzeug version till
maxcountryman/flask-login#686 is fixed.

Signed-off-by: Michal Konečný <[email protected]>
@ThiefMaster
Copy link

It could either be as simple as putting an upper-bound in flask-login

I don't use flask-login, but this kind of upper bound is incredibly annoying:

  • if there's a werkzeug update the end user wants to install, they need to wait for a flask-login update
  • even if there is such a flask-login update, it may also contain other changes, new features, etc. so suddenly wanting to update werkzeug requires looking into whatever else changed in flask-login.

all something you can avoid easily by pinning your transitive dependencies...

@potiuk

This comment was marked as off-topic.

@ThiefMaster

This comment was marked as off-topic.

@potiuk

This comment was marked as off-topic.

@potiuk

This comment was marked as off-topic.

Repository owner locked as too heated and limited conversation to collaborators Jul 25, 2022
@davidism davidism changed the title flask-login is incompatible with werkzeug 2.2.0a1 (parse_rule import fails) incompatible with werkzeug 2.2.0, can't import parse_rule Jul 25, 2022
@davidism
Copy link
Collaborator

davidism commented Jul 25, 2022

See #663 which introduced this issue. #657 reported that the login URL contained both the literal next URL as well as extra query args for current view args, like id=34. I think we can just revert that for now. That behavior was introduced by #462.

@davidism
Copy link
Collaborator

The behavior introduced by #462 was incorrect, I'm going to revert that (and by extension #663). 0.6.2 coming soon.

@davidism davidism self-assigned this Jul 25, 2022
@davidism
Copy link
Collaborator

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants