Add Firestore support

[weeco]

I'd like to use Firestore as database for session management.

I saw that Jens-Uwe already worked on Firestore support (see https://github.com/jum/scs/commit/dd23a3e6d1ff63ed70f5020efeea25d01dde50ad), but since there's no PR yet I thought we could track it in a separate issue until something is merged.

[alexedwards]

Hey Martin :wave:

I'd be happy to add Firestore support, but I haven't had (I think!) any contact from Jens-Uwe and I don't know whether they have any interest (or it's permitted) for their work to be merged.

@jum Do you have any interest in your fork of SCS (with Firestore support) being merged with the upstream repository?

Thanks!

[jum]

@alexedwards Indeed I would be very interested in getting it back into your master repo. I did back at that time contact you on that issue, and it was deemed too intrusive as it requires a change to the store interface. Any call there needs a context argument to be able to work with firestore, and I had to bump the major version to be able to change the API. But I hope I did it in a minimally intrusive way so old session store code could be used unchanged with just one generic wrapper interface.

As this is the first bigger go module that I did fork, I had to fight the go module system quite a bit to make it work. So I believe I may have to cleanup my source to be able submit a clean pull request?

[weeco]

Adding a context to all functions sounds reasonable for me, but is it actually required to add Firestore support? Maybe we can add Firestore first and then make the breaking change for the context support in a second step? What do you suggest @alexedwards ?

[jum]

Unfortunately that will not work. firestore requires a current request context as the first argument to each call, without that change it cannot work.Am 14.06.2021 11:27 schrieb Martin Schneppenheim @.***>: Adding a context to all functions sounds reasonable for me, but is it actually required to add Firestore support? Maybe we can add Firestore first and then make the breaking change for the context support in a second step? What do you suggest @alexedwards ?

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.

[weeco]

@jum Why is that required? You could just use a background context within scs no? Similiar like: https://github.com/alexedwards/scs/blob/85ec2fab6bdfcfd83e5b2520195e1a181ea75542/pgxstore/pgxstore.go#L40

[jum]

Well, it was required in my case as I was using still the go1.11 runtime on appengine, which really has some special things attached to the incoming context. With the newer runtimes it might be possible to make it work without the context from the request, I had no need to try that (I already made the incompatible change for my fork).

[jum]

I have attempted to consolidate my fork to one more easily to review pull request. Please add you comments here:

115

[jum]

Ok one more, this time with the new way of doing context args:

https://github.com/alexedwards/scs/pull/124

[alexedwards]

OK, we're now at a point where the firestore package should be working nicely with all SCS features, including iteration of sessions. All that remains is to update the README documentation for the package and we should be able to close this off. Thanks for everyone's effort so far on this :+1:

[jum]

The README update is in #128

[alexedwards]

Thank you!