Ability to run both SQL and RS migrations #170

omid · 2021-08-30T17:33:58Z

Fixes #154

jxs

Hi! And thanks for this amazing work! ❤️ left some comments, if you feel changing the tests structure to address the new changes takes too much time no worries, we can do it in subsequent PR's :)

jxs · 2021-09-02T16:54:39Z

refinery/tests/migrations/V3__add_brand_to_cars_table.sql

@@ -0,0 +1,2 @@
+ALTER TABLE cars


is the addition of this sql migration on the mod migrations dir on purpose? i.e. to test a dir with both sql and mod migrations, I think since we are now merging the macro do embed migrations it probably makes sense to change the tests to work with both, what do you think?

I did it on purpose, to test both sql and mod. Also agree tests need a revisit!

ok yeah so since the tests are passing and work, we can do it on a subsequent PR if you feel like doing it, and we can merge this one before

jxs · 2021-09-02T17:03:44Z

refinery/tests/mysql.rs

@@ -18,41 +16,46 @@ mod mysql {

    mod embedded {


embedded used to be the .sql migrations but since now everything is embedded this relates with my question above that it probably makes sense to update the tests

examples/modules/migrations/mod.rs

refinery_macros/src/lib.rs

README.md

omid · 2021-09-02T18:42:17Z

Thanks. I agree. There are several tests to test SQL and RS. I'm sure we can merge (remove) some of them. It was hard for me to do this, since I'm kinda new to the source code.

omid · 2021-09-03T09:39:16Z

The issue in GH Actions is related to another package.
Why do we run cargo rustdoc with nightly @jxs?

jxs · 2021-09-03T09:47:28Z

The issue in GH Actions is related to another package.
Why do we run cargo rustdoc with nightly @jxs?

it was needed because of broken_intra_doc_links but probably isn't required anymore.
Sorry I haven't yet replied to the question regarding include_bytes I will do so today still

omid · 2021-09-03T09:57:06Z

@jxs thanks. Checked that. It's not needed to be on nightly anymore. So removed it in this PR.

refinery_macros/src/lib.rs

jxs

LGTM now! thanks!! 🎆

omid · 2021-09-13T20:30:54Z

@jxs I don't like what I have done here :(
It's almost impossible to debug the RS migrations or use auto-completion or any kind of validation before running the code.

“Migration” can be anything, not even SQL. For example, I may want to use refinery to copy files from s3 to blob storage!
Currently, there is no way to set up the required libraries, or have auto-completion or …

Any idea how can we solve it? (I'm trying myself to solve it, may come back with some ideas :/ )

jxs · 2021-09-14T12:03:40Z

Hi, wdym debug? I can understand the auto-completion issues, but those are tooling related. If there's a way to improve them i am all for that, but I don't think that it should block these changes

“Migration” can be anything, not even SQL. For example, I may want to use refinery to copy files from s3 to blob storage!

yeah, but that also happened with mod migrations, you can do whatever you want inside the migration function.

Any idea how can we solve it? (I'm trying myself to solve it, may come back with some ideas :/ )

I can also think about an idea to make it so that when you edit these files rust-analyzer is able to find them in the path

omid · 2021-09-14T12:28:35Z

By debug I mean, to be able to find the problems in the code while writing it. Before running migration.

yeah, but that also happened with mod migrations, you can do whatever you want inside the migration function.

Yes, but in this case, I need to define s3 and blob storage requirements in Cargo.toml.
So the requirements will be in Cargo.toml, but there might be no usage in any code, except the migrations that are not in src.

I can also think about an idea to make it so that when you edit these files rust-analyzer is able to find them in the path

That would be awesome.

jxs · 2021-09-14T14:02:52Z

By debug I mean, to be able to find the problems in the code while writing it. Before running migration.

but how is this different from previous mod_migrations! ?

Yes, but in this case, I need to define s3 and blob storage requirements in Cargo.toml.
So the requirements will be in Cargo.toml, but there might be no usage in any code, except the migrations that are not in src.

and the compiler/clippy complains about that?

omid · 2021-09-14T14:08:55Z

but how is this different from previous mod_migrations! ?

Oh, it isn't? So I'm fine with this PR then :) Seems like another topic then

and the compiler/clippy complains about that?

I still don't know. Shouldn't be the case.

jxs · 2021-09-14T14:25:03Z

Oh, it isn't? So I'm fine with this PR then :) Seems like another topic then

I don't know ehe, so far the only downside from introducing this PR seems to be that auto-completion no longer works on the migration files right? Tbh I think that's a minor issue, which can be addressed later

I still don't know. Shouldn't be the case.

aha then it isn't a problem right?

omid · 2021-09-14T14:53:46Z

OK, so all good 👍🏼

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

* refinery_cli: re-introduce MigrationType: and use it on `find_migration_files` to fix the bug introduced on #170 where refinery_cli tries to embed all kinds of files not only .sql * tests: update test suite to match new merged macros: remove redundant tests, closes #171 * ci: update rust stable version * ci: use github barrel fork to address TLS issues, while a proper version with SqlServer support isn't released

elbart · 2021-10-12T19:19:51Z

Hey! I am using refinery and just found out, that this PR is not yet in some publicly released crates.io version. @jxs are you planning to release that soon or is it possible to directly depend on the github crate?

jxs · 2021-10-15T16:51:30Z

Hi! Yeah I'll try to cut a release today with all the new changes :)
Thanks for your interest!

jxs · 2021-10-16T20:19:00Z

0.7.0 released :)

elbart · 2021-10-16T21:20:23Z

@jxs awesome, thank you so much for all your work!

Ability to run both SQL and RS migrations

6c192f0

omid mentioned this pull request Aug 30, 2021

Feature request: abiliy to use both .sql and .rs migrations #154

Closed

omid added 3 commits August 30, 2021 19:38

Remove mod.rs in example

f5386c0

Fix the documentation

d159fbf

Fix some typos + check CLI

b453df6

omid marked this pull request as ready for review August 30, 2021 18:01

fmt

478cf28

jxs requested changes Sep 2, 2021

View reviewed changes

Remove one useless example structure

33e3c77

Run rustdoc on stable

e427776

omid added 2 commits September 3, 2021 12:23

next try

a325f32

Force recompile in case a migration changed

196b49b

jxs reviewed Sep 5, 2021

View reviewed changes

refinery_macros/src/lib.rs Outdated Show resolved Hide resolved

Update refinery_macros/src/lib.rs

d2d3fab

jxs approved these changes Sep 5, 2021

View reviewed changes

jxs merged commit 5de8d03 into rust-db:main Sep 5, 2021

jxs mentioned this pull request Sep 5, 2021

update test suit to match #170 #171

Closed

omid deleted the merge_macros branch September 5, 2021 16:26

jxs added a commit to jxs/refinery that referenced this pull request Sep 17, 2021

refinery_cli: re-introduce MigrationType:

063d572

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs added a commit to jxs/refinery that referenced this pull request Sep 18, 2021

refinery_cli: re-introduce MigrationType:

53f790b

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs added a commit to jxs/refinery that referenced this pull request Sep 18, 2021

refinery_cli: re-introduce MigrationType:

801d595

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs added a commit to jxs/refinery that referenced this pull request Sep 18, 2021

refinery_cli: re-introduce MigrationType:

2e3db95

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs added a commit to jxs/refinery that referenced this pull request Sep 18, 2021

refinery_cli: re-introduce MigrationType:

819bba4

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs added a commit to jxs/refinery that referenced this pull request Sep 18, 2021

refinery_cli: re-introduce MigrationType:

fa7c792

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs added a commit to jxs/refinery that referenced this pull request Sep 18, 2021

refinery_cli: re-introduce MigrationType:

db3c6a5

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs added a commit to jxs/refinery that referenced this pull request Sep 18, 2021

refinery_cli: re-introduce MigrationType:

4b482ba

and use it on `find_migration_files` to fix the bug introduced on rust-db#170 where refinery_cli tries to embed all kinds of files not only .sql

jxs mentioned this pull request Sep 21, 2021

make .rs migrations detectable by the project #183

Open

jxs mentioned this pull request Oct 2, 2021

No migrations to apply #116

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to run both SQL and RS migrations #170

Ability to run both SQL and RS migrations #170

omid commented Aug 30, 2021 •

edited

Loading

jxs left a comment •

edited

Loading

jxs Sep 2, 2021

omid Sep 2, 2021

jxs Sep 2, 2021

jxs Sep 2, 2021

omid commented Sep 2, 2021

omid commented Sep 3, 2021

jxs commented Sep 3, 2021

omid commented Sep 3, 2021

jxs left a comment

omid commented Sep 13, 2021

jxs commented Sep 14, 2021

omid commented Sep 14, 2021

jxs commented Sep 14, 2021

omid commented Sep 14, 2021

jxs commented Sep 14, 2021

omid commented Sep 14, 2021

elbart commented Oct 12, 2021

jxs commented Oct 15, 2021

jxs commented Oct 16, 2021

elbart commented Oct 16, 2021

Ability to run both SQL and RS migrations #170

Ability to run both SQL and RS migrations #170

Conversation

omid commented Aug 30, 2021 • edited Loading

jxs left a comment • edited Loading

Choose a reason for hiding this comment

jxs Sep 2, 2021

Choose a reason for hiding this comment

omid Sep 2, 2021

Choose a reason for hiding this comment

jxs Sep 2, 2021

Choose a reason for hiding this comment

jxs Sep 2, 2021

Choose a reason for hiding this comment

omid commented Sep 2, 2021

omid commented Sep 3, 2021

jxs commented Sep 3, 2021

omid commented Sep 3, 2021

jxs left a comment

Choose a reason for hiding this comment

omid commented Sep 13, 2021

jxs commented Sep 14, 2021

omid commented Sep 14, 2021

jxs commented Sep 14, 2021

omid commented Sep 14, 2021

jxs commented Sep 14, 2021

omid commented Sep 14, 2021

elbart commented Oct 12, 2021

jxs commented Oct 15, 2021

jxs commented Oct 16, 2021

elbart commented Oct 16, 2021

omid commented Aug 30, 2021 •

edited

Loading

jxs left a comment •

edited

Loading