search

alexedwards / scs

HTTP Session Management for Go
MIT License
2.09k stars 166 forks source link

Store interface methods should accept ctx #81

Closed alexedwards closed 2 years ago

alexedwards commented 4 years ago

All Store interface methods should take a context.Context as the first parameter. The SQL-based stores should also then pass this on when making a database query.

jum commented 4 years ago

I am working in my fork github.com/jum/scs on this issue. I started with this (I hope minimally invasive) change to introduce a ContextStore and StoreAdapter for the V2 Store interface:

https://github.com/jum/scs/commit/542e1e937bea2b763cb5c785e8ba2a9a55bde5f8

I also added a Google Cloud Firestore store implementation to test this:

https://github.com/jum/scs/commit/dd23a3e6d1ff63ed70f5020efeea25d01dde50ad

What do you think about this approach?

jum commented 2 years ago

I believe this issue is done now, with the new *Ctx() methods. But it would be nice to be able to tag a new minor version to be able to use all these new feature without the need to use go.mod replace directives.

alexedwards commented 2 years ago

@jum I totally agree. I want to check and tidy up a few things, but releasing a new minor version is a priority of mine for next week.

alexedwards commented 2 years ago

v2.5.0 is now released including this : )

jum commented 2 years ago

Thank you! Just updated the first two projects to use your original scs again, dropping my branch. I also tested firestore and the iteration via AllCtx works fine.