Switch from beaker to flask-session #7893
Conversation
Co-authored-by: Patricio Del Boca <patriciodelboca@gmail.com>
|
@smotornyuk this one is looking good! and I would love to merge it. What was the decision on the Tech Team? Are we going to provide support for old config names? |
|
Here's an update for the Redis backend. flask-sqlalchemy expects the |
Co-authored-by: Patricio Del Boca <patriciodelboca@gmail.com>
|
Seems okay @smotornyuk ! I'm just adding @wardi and @amercader for an extra pair of eyes. |
|
any update on this ? the beaker cache sessions exploding is killing my server. |
This involved two internal changes: 1. The import path for `RedisSessionInterface` changed 2. The __init__ parameters for `RedisSessionInterface` changed, now requiring `app` to be passed first (see [1]) [1] pallets-eco/flask-session@d30039a#diff-7c52990e5016192ede98e2b9e699d49ef84f7c6735bd10eb9b8e5eba4758c810
ckan/config/middleware/flask_app.py
Outdated
| option_name = f"ckan.session.custom_type.{session_type}.import_path" | ||
|
|
||
| if path := config.get(option_name): | ||
| return import_string(path)(app) |
There was a problem hiding this comment.
Importing arbitrary python code from the INI/environment variables doesn't align with ckan's current pattern of implementing interfaces and enabling plugins. Is there a reason for this different approach here?
There was a problem hiding this comment.
If there isn't an immediate need for this interface I suggest we drop it and discuss the best way to implement pluggable session types. import_string(s) is roughly the same as exec(s) so I'm leery about including it here.
There was a problem hiding this comment.
Yes, perhaps adding support for custom interfaces is premature. The whole point of Flask-Session is that provides many options already out of the box.
I was also going to suggest that we remove the ckan.session.custom_type.cookie.import_path and ckan.session.custom_type.redis.import_path options. The wrappers @smotornyuk introduced make perfect sense in the context of CKAN, I don't think they need to be customizable.
|
@smotornyuk if you can have a quick look at 4325d3a to make sure I haven't missed anything that would be great. I moved the session interface classes to another module to avoid circular import errors. |
|
Everything is good. There is a mention of custom interfaces in changelog: "See :ref: |
Remove beaker and use flask + flask-session instead