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

Add support for SQL isolation levels #812

Merged
merged 7 commits into from
Jun 19, 2018
Merged

Conversation

iand675
Copy link
Contributor

@iand675 iand675 commented Jun 1, 2018

This adds support for assorted SQL isolation levels to all of the standard SQL backends.

There is a slight breaking change for any dependent SQL backends in that the connBegin function now takes an optional isolation level argument. This is necessary because for MySQL the transaction isolation level must be specified before calling begin.

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

@paul-rouse
Copy link
Contributor

I don't feel very competent to approve this, especially for postgresql which I haven't used for years, but, that said, I've looked through the changes and they seem to be fine. If nobody else steps in with comments, I'll go ahead in a few days.

@iand675
Copy link
Contributor Author

iand675 commented Jun 19, 2018

Hi @paul-rouse, any chance you're ready to merge this yet?

@bitemyapp
Copy link
Contributor

I'll take a look at this as I'm an active Persistent + PostgreSQL user.

@paul-rouse
Copy link
Contributor

Thanks @bitemyapp !

@bitemyapp
Copy link
Contributor

This is a very neat PR. Thank you @iand675, good seeing you around too :)

@bitemyapp bitemyapp merged commit 53fa758 into yesodweb:master Jun 19, 2018
@iand675
Copy link
Contributor Author

iand675 commented Jun 27, 2018

@bitemyapp Glad to you like it! Any notion on when it'll go up on hackage? Waiting to merge the corresponding yesod PRs til this is all the way shipped.

@bitemyapp
Copy link
Contributor

@iand675 I'll take a sweep of open PRs this weekend, see what can make it in.

naushadh added a commit to naushadh/persistent that referenced this pull request Jul 4, 2018
@ProofOfKeags
Copy link

Quickly wanted to check in and see if this can go to hackage/stackage. Otherwise would you recommend pinning this hash?

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.

4 participants