Feature/transaction options #1924
Conversation
|
@andrewwhitehead would you prefer to finish your PR or just see this one merged? I would credit you both in the changelog. |
|
I'm happy with merging this one, thanks! |
Signed-off-by: Andrew Whitehead <[email protected]>
Signed-off-by: Andrew Whitehead <[email protected]>
Signed-off-by: Andrew Whitehead <[email protected]>
Signed-off-by: Andrew Whitehead <[email protected]>
Signed-off-by: Andrew Whitehead <[email protected]>
Signed-off-by: Andrew Whitehead <[email protected]>
xfbs
force-pushed
the
feature/transaction-options
branch
from
3672b13 to
d9987bd
Compare
|
FYI, I've just rebased on top of the current main branch and pushed the changes. Hope I didn't break anything. |
|
PR target changed to the 0.7 development branch. @xfbs can you rebase again? |
|
I'll rebase this later today, iirc it's also missing some tests. |
|
@xfbs : I've rebased it already in my local temp branch: https://github.com/madadam/sqlx/commits/patches. Feel free to use it. |
|
Any updates? :) |
|
@xfbs I finally finished my massive refactor, do you mind rebasing one more time? |
|
Hey Austin,
I can try! I have to say that I might need a little help with rebasing,
since a lot of stuff has changed.
Cheers, Patrick
…On Wed 1. Feb 2023 at 19:57, Austin Bonander ***@***.***> wrote:
@xfbs <https://github.com/xfbs> I finally finished my massive refactor,
do you mind rebasing one more time?
—
Reply to this email directly, view it on GitHub
<#1924 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAF775AIN74WVGOIGX5UEMTWVMBATANCNFSM5ZQXLO5A>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
abonander
force-pushed
the
main
branch
2 times, most recently
from
6cf15b0 to
eade49c
Compare
|
Closing due to inactivity. Feel free to reopen if you get a chance to come back to this. |
| /// Begin a new transaction or establish a savepoint within the active transaction. | ||
| /// | ||
| /// Returns a [`Transaction`] for controlling and tracking the new transaction. |
There was a problem hiding this comment.
I would say something about options.
| where | ||
| Self: Sized, | ||
| { | ||
| Self::begin_with(self, Default::default()) |
There was a problem hiding this comment.
Why not take the same approach for the other impls (like impl Acquire for Pool)?
| @@ -43,10 +55,14 @@ impl TransactionManager for MySqlTransactionManager { | |||
| fn rollback(conn: &mut MySqlConnection) -> BoxFuture<'_, Result<(), Error>> { | |||
| Box::pin(async move { | |||
| let depth = conn.transaction_depth; | |||
| @@ -62,13 +67,14 @@ impl<'c, DB> Transaction<'c, DB> | |||
| where | |||
| DB: Database, | |||
| { | |||
| pub(crate) fn begin( | |||
| pub(crate) fn begin_with( | |||
There was a problem hiding this comment.
I would keep begin to break less user code.
| } | ||
| } | ||
|
|
||
| /// Transaction behaviors for SQLite. |
There was a problem hiding this comment.
Might be worth linking to https://sqlite.org/lang_transaction.html and/or https://www.sqlite.org/isolation.html
| SQLITE_TXN_NONE => SqliteTransactionState::None, | ||
| SQLITE_TXN_READ => SqliteTransactionState::Read, | ||
| SQLITE_TXN_WRITE => SqliteTransactionState::Write, | ||
| _ => return Err(Error::Protocol("Invalid transaction state".into())), |
There was a problem hiding this comment.
Might want to format the value into the error message to make debugging easier.
I've stolen the code from #1827, implemented the suggestions and rebased it on top of the current
mainbranch. Missing some unit tests for the Any-related code.