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

removed createdby attributes and enforced expiry time #13

Merged
merged 1 commit into from
Dec 22, 2020

Conversation

No9
Copy link
Collaborator

@No9 No9 commented Dec 22, 2020

This PR removes the feature to expire sessions based on the created_time plus interval.
The feature was designed to enable the duration of a session to be controlled by database configuration rather than potentially each application instance.
While created_time plus interval is a valid use case as it ensures that all instances of an application service have the same session duration the complexity of mixing the functionality with the default application level expiry_at functionality was error prone due to the index management required and added a great deal of complexity to the API in this library.
Specifically:
This library now creates an index on the expiry_at automatically in order to improve developer experience. If we want to apply a different index to support created_time then we have to remove the expiry_at index and ensure it isn't reapplied.
This would mean that DX would suffer as some iniitalise semantics would be mandatory.

If there is interest in this feature in the future I would recommend implementing it as a separate session store type rather than including it in MongodbSessionStore.

I've also put in a conditional on expiry_at so that expired sessions that haven't been sweeped wouldn't be returned.

@pepoviola Thanks for trying to facilitate the feature but I think sticking with your main implementation makes sense right now.
If you're OK with it I will merge this in.

@pepoviola
Copy link
Contributor

Hi @No9, agree 👍. Make sense ab improve users expirience.
Thanks!

@No9
Copy link
Collaborator Author

No9 commented Dec 22, 2020

Thanks ill merge and package tomorrow

@No9 No9 merged commit c5af567 into master Dec 22, 2020
@delete-merged-branch delete-merged-branch bot deleted the remove-create-expiry branch December 22, 2020 10:05
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.

2 participants