search

Azure-Samples / ms-identity-python-webapp

A Python web application calling Microsoft graph that is secured using the Microsoft identity platform
MIT License
291 stars 138 forks source link

Flask-Session version #30

Closed KillyMcDead closed 4 years ago

KillyMcDead commented 4 years ago

The requirements.txt needs to be updated to flask_session == 0.3.1 due to https://github.com/fengsp/flask-session/issues/119

rayluo commented 4 years ago

We can not go back to flask_session 0.3.1 because it was not up-to-date and caused lots of noise in this repo's issues history.

I used a similar workaround in my fork, and then one user here suggested an improvement. I've just sent my improvement as a PR to the upstream repo.

That's all I could do, for now. I do not have control on how soon it will be accepted in that repo. If it won't happen within a week or so, we will probably have to bring my workaround back from its retirement. :-/

mattfeltonma commented 4 years ago

Workaround for this is to install the latest version of cachelib. An update was added last month to address the issue where sessions were not committing to disk. While the update was committed to the master branch of the repo, the package on PyPi is still old. I worked around this by adding "-e git+git://github.com/pallets/cachelib.git@master#egg=cachelib" to the requirements.txt.

rayluo commented 4 years ago

Thanks @mattfeltonma for the hint! I've left a comment in the cachelib repo to ping the repo owner.

Meanwhile, "adding -e git+git://github.com/pallets/cachelib.git@master#egg=cachelib to the requirements.txt" remains one of the viable workaround. So we can kind of document it here.

But it is uncomfortable for us to officially add a skip-level requirement "-e git+git://github.com/pallets/cachelib.git@master#egg=cachelib, especially without proper versioning semantic, into our sample code base. If pressed, I would rather temporarily bring my workaround back from its retirement.

mattfeltonma commented 4 years ago

Makes sense @rayluo.

rayluo commented 4 years ago

@KillyMcDead @mattfeltonma That upstream fix (https://github.com/pallets/cachelib/pull/12) was released in its 0.1.1 version 7 hours ago. It will be automatically picked up if you have a new install of this MSAL Web App sample, or you could update your environment by pip install --upgrade cachelib.

Please let us know whether that upgrade would fix this issue.

rayluo commented 4 years ago

Please let us know whether that upgrade would fix this issue.

No news is good news. We are closing this issue. Feel free to reopen if you find new issues.