add postgresql migration #47

seth-shaw-unlv · 2018-06-01T22:13:08Z

GitHub Issue: Islandora/documentation#722

Follow up to /pull/46

What does this Pull Request do?

Adds a postgres support for the Gemini migration.

What's new?

Compares the installed db and issues the appropriate SQL command. Note that it now drops any existing Gemini db, so back it up before you use this.

How should this be tested?

Same as /pull/46 except with postgres!

Pull or apply patch
run composer install inside Gemini
Make sure your working and existing config.yaml file is still there and it works
run php bin/console list to see available commands
run php bin/console migrations:migrate which will trigger a Migration file.

Interested parties

@DiegoPino @Islandora-CLAW/committers

codecov · 2018-06-01T22:15:33Z

Codecov Report

Merging #47 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #47   +/-   ##
=========================================
  Coverage     55.03%   55.03%           
  Complexity      123      123           
=========================================
  Files             5        5           
  Lines           645      645           
=========================================
  Hits            355      355           
  Misses          290      290

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac3ac50...a120ddf. Read the comment docs.

seth-shaw-unlv · 2018-06-01T22:16:46Z

Yay! Travis is happy!

DiegoPino · 2018-06-01T22:38:18Z

Gemini/src/Migrations/Version20180530031926.php

-            $this->connection->getDatabasePlatform()->getName() !== 'mysql',
-            'Migration can only be executed safely on \'mysql\'.'
-        );
+      $this->addSql(


just a heads up here. I agree that at this stage of the project dropping is totally Ok! But, we should avoid dropping if the table is there and full of tiny little URIs.

Agreed. Future version files should do some sort of ETL process. I just thought that it would be way more complex than necessary at this stage. It would be easier for the few people who need it right now to do an external ETL process than to code it here.

I agree! So, probably for a next step (new issue, future) of this module, to add an ETL process console scrip that is not a migration? I mean, something that can read via drupal's rest API all URI, check back to Fedora and is able to reconstruct (in the way Fedora 3 did) the mapping/index? Would be good to have, there are just too many ways a mapper can fail and too many ways a mapper could need to be reconstructed. We will need to support @Natkeeran that has some kind of claw hybrid running in the wild.

Once this gets release we'll def. be on the hook for migrating. At least now we can do it less manually 👍

DiegoPino · 2018-06-01T22:38:43Z

Gemini/src/Migrations/Version20180530031926.php

-            dateCreated DATETIME NOT NULL, dateUpdated DATETIME NOT NULL, 
-            UNIQUE KEY(fedora_hash, drupal_hash), PRIMARY KEY(uuid)) 
-            DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci ENGINE = InnoDB'
+          'CREATE UNIQUE INDEX fedora_drupal_hash ON Gemini (fedora_hash, drupal_hash);'


DiegoPino · 2018-06-01T22:41:48Z

@seth-shaw-unlv cool! I don't see the approve workflow enabled here but you have my blessing. @daniel-dgi do you see any conflict of committers workflow if I merge? Since some of the code came is == to comment in another pull? @seth-shaw-unlv version is cleaner of course!!

In any case, I would feel more comfortable if you or @whikloj or @jonathangreen could test too and/or merge.

seth-shaw-unlv · 2018-06-01T22:47:20Z

I would feel better if @whikloj, @dannylamb, or @jonathangreen pulled the trigger here.

DiegoPino

Looks good

DiegoPino · 2018-06-01T22:50:56Z

Gemini/src/Migrations/Version20180530031926.php

-        $this->abortIf(
-            $this->connection->getDatabasePlatform()->getName() !== 'mysql',
-            'Migration can only be executed safely on \'mysql\'.'
-        );


Oh, last comment. This abortIf should stay in an extra "else" at the bottom, we don't want that folks with Oracle or Ms SQL Server have a false notion that their tables were created because they did not get any feedback from running the script

And say... Migration can only be executed safely on 'mysql' or 'postgres'

and without the `this->connection->getDatabasePlatform()->getName() !== because we already checked for that in the if and the ifelse right? so basically a $this->abortIf(TRUE, $message)?

Added. (See below.)

dannylamb · 2018-06-04T16:55:24Z

@seth-shaw-unlv My current box is mysql. Let me spin up an ubuntu with postgres and give this a whirl.

dannylamb · 2018-06-05T14:11:57Z

My most recent install with postgresql bombed when installing with drush. I'm going to give it another shot and see if it'll straighten itself out.

dannylamb · 2018-06-05T15:47:46Z

@seth-shaw-unlv @jonathangreen @whikloj I can't seem to get postgres up for the life of me. Keep running into this when trying to use drush to install the site:

TASK [geerlingguy.drupal : Install Drupal with drush.] *************************
Tuesday 05 June 2018  11:58:08 -0300 (0:00:00.491)       0:32:18.133 ********** 
fatal: [default]: FAILED! => {"changed": true, "cmd": ["/var/www/html/drupal/vendor/drush/drush/drush", "site-install", "standard", "-y", "--root=/var/www/html/drupal/web", "--site-name=Islandora-CLAW", "--account-name=admin", "--account-pass=islandora", "--db-url=pgsql://drupal8:islandora@127.0.0.1/drupal8"], "delta": "0:00:00.544881", "end": "2018-06-05 14:58:09.640837", "msg": "non-zero return code", "rc": 1, "start": "2018-06-05 14:58:09.095956", "stderr": " [error]  Failed to create database: psql: FATAL:  password authentication failed for user \"drupal8\"FATAL:  password authentication failed for user \"drupal8\"password retrieved from file \"/tmp/drush_l3R5LK\" ", "stderr_lines": [" [error]  Failed to create database: psql: FATAL:  password authentication failed for user \"drupal8\"FATAL:  password authentication failed for user \"drupal8\"password retrieved from file \"/tmp/drush_l3R5LK\" "], "stdout": "\n // You are about to DROP all tables in your 'drupal8' database. Do you want to \n // continue?: yes.                                                             ", "stdout_lines": ["", " // You are about to DROP all tables in your 'drupal8' database. Do you want to ", " // continue?: yes.                                                             "]}

Trying to use the username of drupal8 seems a bit weird, but I'm not sure. We also may be in a weird in-between state because Islandora/documentation#86 got merged but the install hasn't been updated.

I'm trying one more time, this time running ansible manually instead of hacking up my vagrant file to add the pgsql var.

ansible-playbook -i inventory/vagrant playbook.yml -e "claw_db=pgsql" -e "islandora_distro=ubuntu/xenial64"

If that doesn't work, maybe I should address Islandora-Devops/islandora-playbook#66 first.

seth-shaw-unlv · 2018-06-05T15:56:13Z

@dannylamb Oh, yeah, I remember seeing that... it was a configuration issue where the password wasn't being set properly. I had a fix for it, but where did I put it...

I have to run to a meeting right now, but I can look into it when I get back.

dannylamb · 2018-06-05T16:09:22Z

@seth-shaw-unlv ok, cool. i'll work on Islandora-Devops/islandora-playbook#66 and circle back to this.

seth-shaw-unlv · 2018-06-05T17:25:25Z

Too many tabs open...

@dannylamb We have three config files at play here: database.yml, all/passwords.yml, and webserver/drupal.yml.

Geerlingguy's install drupal task uses the variables drupal_db_user and drupal_db_password. drupal_db_password is hardcoded in the playbook in webserver/drupal.yml and password in all/passwords. Yet, database.yml only establishes hardcodes a root user.

The easy fix is to replace the postgres username in database.yml by using the line -name: "{{ drupal_db_user }}" instead of -name: root.

dannylamb · 2018-06-05T18:12:53Z

Trying that out now. If it works I'll issue the PR.

whikloj · 2018-06-05T18:47:28Z

@seth-shaw-unlv your gonna need to rebase this, sorry 😢

dannylamb · 2018-06-05T19:06:37Z

@seth-shaw-unlv That postgres fix appeared to work for me. As soon as the environment finishes provisioning I'll test this out.

dannylamb · 2018-06-06T01:32:27Z

🆗

ubuntu@claw:/var/www/html/Crayfish/Gemini$ psql -d gemini -h 127.0.0.1 -U crayfish -W
Password for user crayfish: 
psql (9.5.13)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, bits: 256, compression: off)
Type "help" for help.

gemini=> select * from gemini;
 id | uuid | drupal | fedora 
----+------+--------+--------
(0 rows)

gemini=> \q
ubuntu@claw:/var/www/html/Crayfish/Gemini$ php bin/console migrations:migrate
Loading configuration from the integration code of your framework (setter).
                                                         
                    Gemini Migrations                    
                                                         

WARNING! You are about to execute a database migration that could result in schema changes and data lost. Are you sure you wish to continue? (y/n)y
Migrating up to 20180530031926 from 0

  ++ migrating 20180530031926

     -> DROP TABLE IF EXISTS Gemini;
     -> CREATE TABLE Gemini (
             fedora_hash VARCHAR(128) NOT NULL,
             drupal_hash VARCHAR(128) NOT NULL,
             uuid VARCHAR(36) PRIMARY KEY,
             drupal_uri TEXT NOT NULL,
             fedora_uri TEXT NOT NULL,
             dateCreated TIMESTAMP NOT NULL,
             dateUpdated TIMESTAMP NOT NULL
           );
     -> CREATE UNIQUE INDEX fedora_drupal_hash ON Gemini (fedora_hash, drupal_hash);

  ++ migrated (0.14s)

  ------------------------

  ++ finished in 0.14s
  ++ 1 migrations executed
  ++ 3 sql queries
ubuntu@claw:/var/www/html/Crayfish/Gemini$ psql -d gemini -h 127.0.0.1 -U crayfish -W
Password for user crayfish: 
psql (9.5.13)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, bits: 256, compression: off)
Type "help" for help.

gemini=> select * from gemini;
 fedora_hash | drupal_hash | uuid | drupal_uri | fedora_uri | datecreated | dateupdated 
-------------+-------------+------+------------+------------+-------------+-------------
(0 rows)

Then after adding a collection:

gemini=> select * from gemini;
                                                           fedora_hash                                                            |                                                           drupal_hash                                                            |                 uuid                 |                 drupal_uri                  |                                     fedora_uri                                     |     datecreated     |     dateupdated     
----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+--------------------------------------+---------------------------------------------+------------------------------------------------------------------------------------+---------------------+---------------------
 a841ae93e7d5124d64adab2452e7cbedeb52b16c6647b1eb8c9fce7098587de3a18b8be26c658ccc515b8bb0d5760fd0106a1a98986935b0c58c6b36d95f677a | 9e3a4d05bd0bbd6d558bffe43759511425096412ef0ba523201a82b2a0fbf934ecb84d011d01f3866187387135f567b9e82e0dd19bb2ae35407e17814fbe771a | 2f87766d-9d28-4484-a66c-4647779332ab | http://localhost:8000/node/1?_format=jsonld | http://localhost:8080/fcrepo/rest/2f/87/76/6d/2f87766d-9d28-4484-a66c-4647779332ab | 2018-06-05 20:23:03 | 2018-06-05 20:23:03
(1 row)

Looks good to me 👍

add postgresql migration

20df1da

seth-shaw-unlv mentioned this pull request Jun 1, 2018

use php console migrations:migrate to install gemini table islandora-deprecated/ansible-role-crayfish#9

Merged

DiegoPino reviewed Jun 1, 2018

View reviewed changes

DiegoPino approved these changes Jun 1, 2018

View reviewed changes

DiegoPino reviewed Jun 1, 2018

View reviewed changes

abort gemini table migration if db is not supported

e82c612

Merge branch 'master' into issue-828

a120ddf

dannylamb merged commit ed840f7 into master Jun 6, 2018

seth-shaw-unlv deleted the issue-828 branch June 6, 2018 04:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add postgresql migration #47

add postgresql migration #47

seth-shaw-unlv commented Jun 1, 2018

codecov bot commented Jun 1, 2018 •

edited

Loading

seth-shaw-unlv commented Jun 1, 2018

DiegoPino Jun 1, 2018

seth-shaw-unlv Jun 1, 2018

DiegoPino Jun 1, 2018

dannylamb Jun 4, 2018

DiegoPino Jun 1, 2018

DiegoPino commented Jun 1, 2018 •

edited

Loading

seth-shaw-unlv commented Jun 1, 2018

DiegoPino left a comment

DiegoPino Jun 1, 2018 •

edited

Loading

DiegoPino Jun 1, 2018

DiegoPino Jun 1, 2018

seth-shaw-unlv Jun 1, 2018

dannylamb commented Jun 4, 2018

dannylamb commented Jun 5, 2018 •

edited

Loading

dannylamb commented Jun 5, 2018

seth-shaw-unlv commented Jun 5, 2018 •

edited

Loading

dannylamb commented Jun 5, 2018

seth-shaw-unlv commented Jun 5, 2018 •

edited

Loading

dannylamb commented Jun 5, 2018

whikloj commented Jun 5, 2018

dannylamb commented Jun 5, 2018

dannylamb commented Jun 6, 2018

add postgresql migration #47

add postgresql migration #47

Conversation

seth-shaw-unlv commented Jun 1, 2018

What does this Pull Request do?

What's new?

How should this be tested?

Interested parties

codecov bot commented Jun 1, 2018 • edited Loading

Codecov Report

seth-shaw-unlv commented Jun 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoPino commented Jun 1, 2018 • edited Loading

seth-shaw-unlv commented Jun 1, 2018

DiegoPino left a comment

Choose a reason for hiding this comment

DiegoPino Jun 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dannylamb commented Jun 4, 2018

dannylamb commented Jun 5, 2018 • edited Loading

dannylamb commented Jun 5, 2018

seth-shaw-unlv commented Jun 5, 2018 • edited Loading

dannylamb commented Jun 5, 2018

seth-shaw-unlv commented Jun 5, 2018 • edited Loading

dannylamb commented Jun 5, 2018

whikloj commented Jun 5, 2018

dannylamb commented Jun 5, 2018

dannylamb commented Jun 6, 2018

codecov bot commented Jun 1, 2018 •

edited

Loading

DiegoPino commented Jun 1, 2018 •

edited

Loading

DiegoPino Jun 1, 2018 •

edited

Loading

dannylamb commented Jun 5, 2018 •

edited

Loading

seth-shaw-unlv commented Jun 5, 2018 •

edited

Loading

seth-shaw-unlv commented Jun 5, 2018 •

edited

Loading