search

pallets-eco / flask-session

Server side session extension for Flask
https://flask-session.readthedocs.io
BSD 3-Clause "New" or "Revised" License
488 stars 236 forks source link

Version 0.7.0 is a breaking change #223

Closed eiriklid closed 3 months ago

eiriklid commented 3 months ago

Our builds are failing as 'cachelib' have been taken out of the dependencies.

...
root/.tox/py/lib/python3.10/site-packages/flask_session/__init__.py:133: in _get_interface
    from .filesystem import FileSystemSessionInterface
/root/.tox/py/lib/python3.10/site-packages/flask_session/filesystem/__init__.py:1: in <module>
    from .filesystem import FileSystemSession, FileSystemSessionInterface  # noqa: F401
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
    import warnings
    from datetime import timedelta as TimeDelta
    from typing import Optional

>   from cachelib.file import FileSystemCache
E   ModuleNotFoundError: No module named 'cachelib'
Lxstr commented 3 months ago

Hi @eiriklid sorry it does seem I missed including this in the change log and probably should have done it in the 1.0 release.

Although changing dependencies isn't technically a breaking API change in semver and the need to install the client you are using is noted in the updated docs, I'm thinking I will release a patch change for this and remove it again in 1.0.0.

For now can you add cachelib to your required dependencies?

You will need to do this anyway for 1.0.0 if you choose to update and also heads up that filesystem will be removed in 1.0.0 for the cachelib back end which opens up more options.

eiriklid commented 3 months ago

I would usually expect dependencies of a package to be listed. If it is optional, as I suspect it will be in 1.0.0, then I would expect it to be listed as an optional dependency. https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#optional-dependencies

If not we and the dependency resolution system would have guess what the dependencies are.

If you want to @Lxstr, I can try to create this in a PR.

Lxstr commented 3 months ago

@eiriklid yeah fair point. I would appreciate the PR. Just note that Flask session actually supports a number of memcache clients and I'm not sure how best to handle that. Perhaps we pick the best client to include in the optional dependencies and then note in the docs that you can manually install alternative one if you don't choose that optional one

Lxstr commented 3 months ago

Thanks @eiriklid for the PR which has been merged and released in 0.8.0