-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add SessionMiddleWare Configuration #251
Add SessionMiddleWare Configuration #251
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! Overall it looks great, I just have a single suggestion. Please make secure
to be true
by default, but modify cot-cli/src/project_template/config/dev.toml
and set it to false
there in the config template. This way we keep Cot secure by default and keep development easy.
Also, it'd be great if you could write a few additional tests to bump up the coverage.
Once you make those changes, please make the PR ready for review and we'll give our final comments!
Codecov ReportAttention: Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good, thank you a lot for confirming the issue and for the fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work and once again thank you for your contribution! If you want to work on SessionMiddleware
supporting different stores, please take a crack at it in a separate PR ;)
yes, definitely, would like to take up the admin issues and if no one gets to it before I do, I'd be happy to look into it :) |
This PR fixes a bug as described in #247. It introduces a Session middleware configuration where users can now set session config values either in the
toml
file or using the API. For config knobs supported upstream, this PR goes for an initial best effort of implementing only thesecure
config knob. Others will definitely be added later.for
toml
files, it will typically look like this:using the API: