import migrations and don't require diesel_cli for admins #555

trinity-1686a · 2019-04-28T15:39:19Z

trinity-1686a a commenté

2019-04-28 15:39:19 +00:00

.

Propriétaire

this is an attempt at integrating migrations to plm, with the goal to remove need for diesel_cli, and to allow arbitrary rust in migrations (can come handy for many reasons, like need to upgrade tantivy index version due to upstream changes, without need to take special steps for sysadmins)

backbone for the feature
integration to plm as new sub-command
update ci
make plume fail to launch if it detect migrations must be run

this is an attempt at integrating migrations to plm, with the goal to remove need for diesel_cli, and to allow arbitrary rust in migrations (can come handy for many reasons, like need to upgrade tantivy index version due to upstream changes, without need to take special steps for sysadmins) - [x] backbone for the feature - [x] integration to plm as new sub-command - [x] update ci - [x] make plume fail to launch if it detect migrations must be run

👍 2

codecov[bot] a commenté

2019-04-28 16:38:47 +00:00

. (Migré de github.com)

Codecov Report

Merging #555 into master will increase coverage by 0.3%.
The diff coverage is 89.91%.

@@            Coverage Diff            @@
##           master     #555     +/-   ##
=========================================
+ Coverage   44.08%   44.39%   +0.3%     
=========================================
  Files          68       70      +2     
  Lines        7937     8044    +107     
=========================================
+ Hits         3499     3571     +72     
- Misses       4438     4473     +35

# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/555?src=pr&el=h1) Report > Merging [#555](https://codecov.io/gh/Plume-org/Plume/pull/555?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/787eb7f3992e43e7a149288f3378c90dd4fe4bf8?src=pr&el=desc) will **increase** coverage by `0.3%`. > The diff coverage is `89.91%`. ```diff @@ Coverage Diff @@ ## master #555 +/- ## ========================================= + Coverage 44.08% 44.39% +0.3% ========================================= Files 68 70 +2 Lines 7937 8044 +107 ========================================= + Hits 3499 3571 +72 - Misses 4438 4473 +35 ```

elegaanz (Migré de github.com) a évalué cette demande d’ajout 2019-04-28 17:00:27 +00:00

script/run_unit_test.sh

elegaanz (Migré de github.com) a commenté

2019-04-28 17:00:26 +00:00

.

Isn't this always true? (maybe not, I'm really bad with Bash)

trinity-1686a a évalué cette demande d’ajout 2019-04-28 17:15:47 +00:00

script/run_unit_test.sh

trinity-1686a a commenté

2019-04-28 17:15:47 +00:00

.

Auteur

Propriétaire

oops I tested with a value, and forgot to use the variable after 😬

oops I tested with a value, and forgot to use the variable after :grimacing:

igalic (Migré de github.com) a évalué cette demande d’ajout 2019-04-29 07:11:31 +00:00

igalic (Migré de github.com) a laissé un commentaire

👀

plume-macro/src/lib.rs

					
				@ -0,0 +48,4 @@

				                .unwrap()

				                .chars()

				                .filter(char::is_ascii_digit)

				                .take(14)

igalic (Migré de github.com) a commenté

2019-04-29 07:09:12 +00:00

.

this seems like a strange thing to take()
what is this whole expression supposed to do?

this seems like a strange thing to `take()` what is this whole expression supposed to do?

trinity-1686a a évalué cette demande d’ajout 2019-04-29 09:51:27 +00:00

plume-macro/src/lib.rs

					
				@ -0,0 +48,4 @@

				                .unwrap()

				                .chars()

				                .filter(char::is_ascii_digit)

				                .take(14)

trinity-1686a a commenté

2019-04-29 09:51:27 +00:00

.

Auteur

Propriétaire

diesel store whether migrations were run by storing a 14 digit code. For 2019-03-06-115158_blog_images, it is translated to 20190306115158. To do that I get an iterator over the folder name, filter only digits, and take the first 14 ones (if there are more, they are ignored). See here for take documentation

diesel store whether migrations were run by storing a 14 digit code. For `2019-03-06-115158_blog_images`, it is translated to `20190306115158`. To do that I get an iterator over the folder name, filter only digits, and take the first 14 ones (if there are more, they are ignored). See [here](https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.take) for `take` documentation

elegaanz (Migré de github.com) a évalué cette demande d’ajout 2019-04-29 09:58:34 +00:00

plume-models/src/migrations.rs

					
				@ -0,0 +49,4 @@

				pub struct ImportedMigrations(&'static [ComplexMigration]);

				impl ImportedMigrations {

				    pub fn run_pending_migrations(&self, conn: &Connection, path: &Path) -> Result<()> {

elegaanz (Migré de github.com) a commenté

2019-04-29 09:58:34 +00:00

.

Should these dbg! be here?

Should these `dbg!` be here?

trinity-1686a a évalué cette demande d’ajout 2019-04-29 10:41:02 +00:00

plume-models/src/migrations.rs

					
				@ -0,0 +49,4 @@

				pub struct ImportedMigrations(&'static [ComplexMigration]);

				impl ImportedMigrations {

				    pub fn run_pending_migrations(&self, conn: &Connection, path: &Path) -> Result<()> {

trinity-1686a a commenté

2019-04-29 10:41:02 +00:00

.

Auteur

Propriétaire

no :x

trinity-1686a a commenté

2019-04-29 12:22:58 +00:00

.

Auteur

Propriétaire

At some point the documentation should also be updated to reflect that change

elegaanz (Migré de github.com) a approuvé ces modifications 2019-04-29 13:43:08 +00:00

elegaanz (Migré de github.com) a laissé un commentaire

I don't think it would be possible, but creating the PostgreSQL database if it doesn't exist would be perfect.

But it already works fine, so you can merge as it is.

👍

I don't think it would be possible, but creating the PostgreSQL database if it doesn't exist would be perfect. But it already works fine, so you can merge as it is. :+1:

trinity-1686a a commenté

2019-04-29 13:48:35 +00:00

.

Auteur

Propriétaire

@BaptisteGelez I think it's possible, I'll try to do it (diesel_cli does that from what I saw in it's code)

Edit: apparently it detect a connection error, assume it means the database don't exist, then connect to the same address but without specifying a database, run a query, and then try to reconnect. Considering the first step, this should probably be done in plm, not in plume-models

@BaptisteGelez I think it's possible, I'll try to do it (diesel_cli does that from what I saw in it's code) Edit: apparently it detect a connection error, assume it means the database don't exist, then connect to the same address but without specifying a database, run a query, and then try to reconnect. Considering the first step, this should probably be done in plm, not in plume-models

igalic a commenté

2019-04-29 13:59:44 +00:00

. (Migré de github.com)

I don't think it would be possible, but creating the PostgreSQL database if it doesn't exist would be perfect.

usually, create db is a more privileged operation than creating anything in that db…
and i sure hope people aren't giving our software that kind of permissions 😅

> I don't think it would be possible, but creating the PostgreSQL database if it doesn't exist would be perfect. usually, create db is a more privileged operation than creating anything in that db… and i sure hope people aren't giving our software that kind of permissions 😅

trinity-1686a a commenté

2019-04-29 14:08:48 +00:00

.

Auteur

Propriétaire

So, shall I try, or do I assume people have a "secure" configuration that don't allow us to do privileged things?

elegaanz a commenté

2019-04-29 14:28:08 +00:00

. (Migré de github.com)

Let's say people have a secure configuration. 😄

Let's say people have a secure configuration. :smile:

Connectez-vous pour rejoindre cette conversation.

Sans évaluateur

elegaanz

Sans jalon

Sans projet

Sans assignation

2 participants

Notifications

Échéance

La date d’échéance est invalide ou hors plage. Veuillez utiliser le format "aaaa-mm-dd".

Aucune échéance n'a été définie.

Dépendances

Aucune dépendance définie.

Référence : Plume/Plume#555

Lignes
Colonnes