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

v0.5.0 #9

Merged
merged 9 commits into from
Dec 14, 2021
Merged

v0.5.0 #9

merged 9 commits into from
Dec 14, 2021

Conversation

peppelinux
Copy link
Member

  • BREAKING CHANGE: oidcop session dump to mongodb as string
  • fix: removed warning on mongodb operations (insert, get, count)
  • feat: updated pymongo up to 4.0.1, retrocompatibility with pymongo 3.x indeed

@peppelinux
Copy link
Member Author

@melanger feel free to put your kindly revision on this

@melanger
Copy link
Contributor

@melanger feel free to put your kindly revision on this

Looks reasonable, but it is a breaking change indeed, because old sessions in the database will cause errors when new version is deployed (because they cannot be read), if I understand it correctly. I recommend adding a changelog note that the session DB has to be cleared before updating.

Why was there a zlib compression but it was removed in the end? I think it might be good to keep a compression. I have read that compressing large fields (which are not accessed in the database directly) may improve performance significantly in some cases. With a larger SATOSA instance, performace might be of great importance.

@peppelinux
Copy link
Member Author

@melanger

regarding the breaking change we may suggest to updated the previous definitions instead of deleting them (just a json.dumps in a for loop)

regarding zlib compression, we may think to add this feature as a global parameter.
We decided to avoid compression because mongodb hanldes very well that dump as it is and this, as clear text, would be easy to be matched with a fulltextsearch

would you like to purpose a PR on the dev branch to enable again zlib as an optional global parameter?

@melanger
Copy link
Contributor

would you like to purpose a PR on the dev branch to enable again zlib as an optional global parameter?

If it works for now, let's keep it that way. I may add it later if I hit performance issues.

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

Successfully merging this pull request may close these issues.

3 participants