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

creating method to change session's table default name #177

Conversation

0syntrax0
Copy link

@0syntrax0 0syntrax0 commented Sep 19, 2023

Some of us already have a sessions table that is used in a different way, just adding a method to use a different table.

Also useful when testing or comparing libraries and results.

creating method to change session's table default name

Co-authored-by: csaavedra-digitalmint <[email protected]>
@alexedwards
Copy link
Owner

I like this idea, and agree that supporting a custom name for the sessions table would be nice.

I don't think this PR works as-is --- it only implements the behavior on the pgxstore package, and I also don't really want to change the definition of the Store interface (which would be a breaking change).

I've opened issue #180 to consider the options further.

@gregwebs
Copy link

I also don't really want to change the definition of the Store interface (which would be a breaking change).

Is this riffing on future designs? Because this PR doesn't change the Store interface. It looks like there was a mistake made in the change to the README. This PR changes the PostgresStore struct by adding a field, which is considered backwards compatible.

When a better design comes through that does work for all backends (I don't know what that would be if you don't want to change the interface), this change is still going to be needed.
At best closing this PR would help to maintain more similar conventions across backends when implementing across all backends. So you can consider thoughtfully reviewing the conventions and merging this change now.

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