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

Rollback on exception #18

Closed
marksteward opened this issue May 28, 2012 · 10 comments
Closed

Rollback on exception #18

marksteward opened this issue May 28, 2012 · 10 comments
Assignees

Comments

@marksteward
Copy link
Member

SQLAlchemy seems to be committing regardless

@ghost ghost assigned marksteward May 29, 2012
@Jonty
Copy link
Member

Jonty commented May 29, 2012

Can we get more info on this? Is this exceptions chucked due to constraint violations? Exceptions due to shit code?

@marksteward
Copy link
Member Author

It seems exceptions thrown before db.session.commit don't stop the changes being saved. This means (for gocardless_start) if payment.bill_url() fails, you've still bought the tickets, but didn't get redirected, so they end up in limbo.

We probably need to add a top-level request exception handler that calls db.session.rollback. Alternatively, it might be sqlalchemy not using transactions correctly with sqlite (haven't tested with psql).

@Jonty
Copy link
Member

Jonty commented May 29, 2012

This sounds like the db connection has autocommit set?

@marksteward
Copy link
Member Author

Probably, I assumed that was Flask's responsibility. So the wrapper also needs to call begin.

@JasperWallace
Copy link
Member

i can test this with psql if it would help?

@marksteward
Copy link
Member Author

Heh, I was just about to do the same. Will leave it if you're confident :)

@JasperWallace
Copy link
Member

done, what exactly is the test case?

there is a db.session.commit() in buy_prepay_tickets so we are commiting before we generate the bill url...

maybe move the commit to just before we return the redirect?

@JasperWallace
Copy link
Member

Looks like this does not happen with postgres so we don't have to worry about it.

Not closeing, but removing from the milestone.

@Jonty
Copy link
Member

Jonty commented Jun 25, 2012

If this doesn't happen with postgres, why aren't we closing it?

@marksteward
Copy link
Member Author

If the same test been run against sqlite, I'm happy to. Will do this tomorrow.

@russss russss closed this as completed Dec 7, 2013
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

No branches or pull requests

4 participants