Skip to content

feat: no tx migration #3181

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

Merged
merged 14 commits into from
Apr 19, 2024
Merged

Conversation

cleverjam
Copy link
Contributor

Does your PR solve an issue?

Yes, this change allows for a way to create migrations that will be executed outside of the postgres transaction block.

It works as described in this comment

fixes #767

😬 accidentally closed #3159 when looking at merge conflicts

Copy link
Collaborator

@abonander abonander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last nit.

@cleverjam
Copy link
Contributor Author

cleverjam commented Apr 11, 2024

could someone help me figure out why this test is failing? I can't reproduce the failure locally with DATABASE_URL=postgres://postgres:password@localhost:55595/sqlx cargo test --no-default-features --features any,postgres,macros,_unstable-all-types,runtime-async-std,tls-native-tls

Where 0.0.0.0:55595->5432/tcp sqlx-postgres_11-1

Edit: it seems this test passed previous builds fine (failures were due to formatting), should we re-run?

@cleverjam
Copy link
Contributor Author

@abonander I have addressed your comment, please take a look when you have a chance

@abonander abonander merged commit 40aef6d into launchbadge:main Apr 19, 2024
@qrilka
Copy link

qrilka commented Apr 20, 2024

@cleverjam @abonander is it expected that there seem to be no documentation update in this PR?

@abonander
Copy link
Collaborator

I generally don't require PR authors to write documentation because I usually end up wanting to edit it heavily anyway.

Migrations need a documentation pass in general, they're barely covered at all to begin with. When I get to it, I'll make sure to mention this feature.

jayy-lmao pushed a commit to jayy-lmao/sqlx that referenced this pull request Jun 6, 2024
* test: add a failing test

* feat: add no_tx to migration struct

* feat: execute migration with no tx block

* fix: expected string literal compilation error

* test: update no tx to content comment

* refactor: use the sql comment instead of file name semantics

* docs: remove no_tx from file format comment

* fix: remove filename matches

* fix: messed up merge

* refactor: dedupe migration

* fix: move comment to where it makes sense

* fix: linter error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to run migrations without a transaction
3 participants