maxcountryman / tower-sessions-stores

🚃 Previously bundled session stores for `tower-sessions`.
https://github.com/maxcountryman/tower-sessions
MIT License
22 stars 7 forks source link

Extend provided SQLx `SessionStore` implementations to allow for more configurability #1

Open Hellrespawn opened 9 months ago

Hellrespawn commented 9 months ago

Right now the schema_name for the SQLX stores is hardcoded to be "tower_sessions", ignoring the database specified in the database URL.

My current Docker setup creates a user with access to only one database. It would be useful to to be able to have it use that specific database.

maxcountryman commented 9 months ago

Thanks for mentioning this.

I would be happy to accept changes to better enable configurability around this.

avdb13 commented 9 months ago

Would with_table_name and with_schema_name suffice or do we want this to be part of the Store::new(pool) signature? Latter is a breaking change but we're still early in development. I vouch for the former.

maxcountryman commented 9 months ago

I'm fine with either approach: it's okay to make breaking changes for a better API. 😁

Hellrespawn commented 9 months ago

You arguably don't need a schema_name at all. By omitting it you would rely on SQLx determining it from the database URL or returning an error. That's the approach async-sqlx-session takes.

I quickly ripped out schema_name on a local copy of the crate, and store.migrate().await.unwrap() Just Works™ if you specify a database in the database url or gives the following error if you don't:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: 
Database(MySqlDatabaseError { code: Some("3D000"), number: 1046, 
message: "No database selected" })', application/src/cli/serve.rs:86:27
maxcountryman commented 7 months ago

I quickly ripped out schema_name on a local copy of the crate, and store.migrate().await.unwrap() Just Works™ if you specify a database in the database url or gives the following error if you don't

You don't need to copy the crate to do this, you can instead implement SessionStore: this is a key aspect of this crate's design. In fact, we fully intend for folks to implement their own stores and expect that our provided implementations will only be used as a convenience.

maxcountryman commented 7 months ago

@czocher I think this also relates your recent PR addressing the Postgres store.

czocher commented 7 months ago

@maxcountryman it does indeed relate. We can possibly remove the schema setting completely and (as @Hellrespawn mentioned) rely on sqlx to select the schema name with the &options=-c search_path=someschemahere URL option.