[Tools] Script to automate LORIS SQL patching #3677

johnsaigle · 2018-05-17T19:13:57Z

This PR has been heavily refactored so a lot of the conversation is outdated.

Overview

The script will display to a user which patches they need to apply, in order. By passing the argument --apply-patches a user can have the update script apply SQL patch files to their database automatically.

Details

PHP_CLI_Helper.class.inc provides some functions for nice output and can be re-used by other scripts. It is also used in [Tools] Ubuntu production environment "bootstrap" script #3981.

The function to retrieve patches is heavily dependent on the way we organize and name SQL files. If the directory structure changes, this functionality of the script is likely to break.

To Test

Run the script on your LORIS! Report anything weird or confusing.

PapillonMcGill · 2018-05-22T11:43:30Z

tools/update.php

+
+/**
+ * Updates source code and dependencies for LORIS. Outputs missing patch files
+ * based on verion information


should have an ended dot (.)

ZainVirani · 2018-05-28T15:40:38Z

tools/update.php

+    );
+    $paths     = $config->getSetting('paths');
+    $loris_root_dir = $paths['base'];
+    $backup_dir     = "/tmp/bkp-LORIS"; // TODO: should this be configurable?


Yes, this should be configurable

This is now configurable

ZainVirani · 2018-05-28T16:02:12Z

tools/update.php

+    $A = $db_config['database'];
+    $u = $db_config['username'];
+    $p = $db_config['password'];
+    $h = $db_config['host'];


descriptive variable names would be better

They are descriptive to the extent that they match the flags for mysql on the command line.

ZainVirani · 2018-05-28T16:10:09Z

tools/update.php

+        $cmd = "mysql -h $h -u $u -p -A $A < $patch";
+        if ($report_only === false) {
+            if (doExec($cmd) === false) {
+                break;


so if I understand correctly, if a patch fails, doExec should spit out the error and this loop will break (and the script will terminate as this is the last action in main())

I think before breaking the rest of the commands should be output to the user along with a message indicating that the rest of the commands were not run. I figure if this didn't happen a user would just try to rerun this script which could make things worse.

@ZainVirani that's a good point. I'll come back to this pending the conversation as to whether this script should even apply patches or if it should just print them.

@ZainVirani I have added this functionality. Thanks for the suggestion!

ZainVirani · 2018-05-28T16:15:03Z

tools/update.php

+    // @codingStandardsIgnoreEnd
+    . PHP_EOL . PHP_EOL;
+sleep(3);
+main();


I think it would be best if the script required a -confirm to actually update, and without it would just spew out some help text (and the newest release version # so users can be sure they want to update), just so this script is never accidentally started

There is now a confirm flag present.

tools/update.php

addressed

These changes have been incorporated

PapillonMcGill · 2018-07-09T14:49:17Z

tools/update.php

+ *
+ * @param string $loris_root_dir The directory where LORIS is installed
+ * @param string $backup_dir     The directory prefix where backups are stored.
+ *                      Always '/tmp/loris_' for now


Not anymore as it is read from the CLI

PapillonMcGill · 2018-07-09T14:50:43Z

tools/update.php

+ * @param string $backup_dir     The directory prefix where backups are stored.
+ *                      Always '/tmp/loris_' for now
+ *
+ * @return bool True if rsync executes successfully, false otherwise


the @param and @return should all end with a dot or not for consistency

PapillonMcGill · 2018-07-09T14:52:21Z

tools/update.php

+    if (empty($tarball_path)) {
+        die('ERROR: Could not download the latest LORIS release.');
+    }
+    $dst_dir = '/tmp/'; // Parent directory for backup and release download


This comment should be updated to remove backup

PapillonMcGill · 2018-07-09T15:01:08Z

tools/update.php

+    }
+    echo "[**] Latest version $version_to is ahead of installed $version_from "
+        . "by $diff_major MAJOR release(s), $diff_minor MINOR release(s)."
+        . PHP_EOL;


This could end up as Latest version 22.0 is ahead of installed 21.3 by 1 MAJOR release(s), -3 MINOR release(s).

This negative minor can be quite confusing. I suggest to remove the minor part when major is > 0.

PapillonMcGill · 2018-07-09T15:09:35Z

tools/update.php

+/**
+ * Apply a list of SQL patch files using exec.  Currently this function is only
+ * called with $report_only = true so patches are never applied. This can be
+ * changed in the future if this functionality is required.


This comment need to be updated

PapillonMcGill · 2018-07-09T15:10:12Z

tools/update.php

+ * @param dictionary $db_config     DB config info. Needed for credentials
+ * @param bool       $apply_patches If true, MySQL commands will be run
+ *
+ * @return bool Always true (for now). False if patches fail to be applied.


tools/update.php

ridz1208 · 2018-07-16T20:31:08Z

tools/update.php

+    $patch_dir           = $loris_root . 'SQL/Release_patches/';
+    $all_release_patches = glob($patch_dir . '*.sql');
+    $patches = [];
+    if ($diff_major > 0 || $diff_minor > 0) {


@johnsaigle most of the time projects will need to apply project specif SQL in between 2 releases to prepare the database, this cannot be supported in this script

this cannot be supported in this script

What specifically can't be supported? I'm not sure I understand.

Assume I'm 2 releases behind, for each release I have PRE and POST custom SQL patches designed for my project (thats usually the case for any project upgrading). how can I run them using your script as follows:

PRE_19.0.sql -> project patches

18.0_To_19.0.sql -> loris patches

POST_19.0.sql -> project patches

PRE_20.0.sql -> project patches

19.0_to_20.0.sql -> loris patches

POST_20.0.sql -> project patches

I didn't know about this process for projects.

If it's just one release you could do

Source PRE patch.

Run script.

Source POST patch.

I'm not sure how to handle the case where a project is more than 1 major release behind. If there is a standard location and naming convention for the files it would probably be possible to do these checks. Is that the case? Do you have any suggestions for how to incorporate this feature?

As discussed in person, the script will be amended to not do SQL patching if the installed LORIS version is more than 1 major version behind the latest release, as it's difficult to automate and likely quite risky.

The script will display a message informing the user that the update process is complete in terms of src code, packages, etc. but that they will need to apply patches manually.

ridz1208 · 2018-07-16T20:32:19Z

tools/update.php

+            . PHP_EOL;
+
+        // Add all patches in Archive/$MAJOR.$MINOR. Everything other needed
+        // command will be in the Release patches which are been added above


the release patch is the concatenation of the patches in Archive why are you readding them here ?

Right... I guess this should be New_patches. I remember at some point we were actually putting new patches in Archive so I based this on that.

I'll update this.

johnsaigle · 2018-08-06T18:59:10Z

TODO: Add single-use patches to update process

johnsaigle · 2018-09-04T20:35:54Z

@ridz1208 @PapillonMcGill Please re-review when you can.

PapillonMcGill · 2018-09-05T12:26:39Z

tools/update.php

+}
+// Require a confirm flag so this script is not accidentally run
+if (!in_array('--confirm', $argv)) {
+    $info = json_decode(getLatestReleaseInfo());


already done at line 79. This line is in extra

tools/update.php

modules/login/ajax/verificationimage.php

stale

johnsaigle · 2018-09-05T19:29:06Z

@PapillonMcGill Thanks, I've incorporated your feedback.

PapillonMcGill

Nice work,

will help greatly to let user update their version.

johnsaigle · 2018-10-01T16:01:49Z

@driusan Could you review this when you get a chance? It's been ready for final review for about a month

ridz1208 · 2018-10-02T13:56:09Z

tools/update.php

+/* Create db connection and get version info. */
+$config    = \NDB_Config::singleton();
+$db_config = $config->getSetting('database');
+$db        = \Database::singleton(


that might be problematic if the release patch requires admin credentials. (generally it shouldn't but all release patches have probably been run as admins to this day) just investigate

I think it depends on the privileges of the SQL user, e.g. whether they can do ALTER, CREATE etc. That'll depend in a large part on the administrator of the system and how they set up their databases.

I think in the worst case MySQL will give an error about not having the right permissions and the script will continue onward which shouldnt cause issues as patch application is the last step.

it still places the DB is a half-update state and thats generally very difficult to recover front. A very Ideal way to tackle this would be to see what privileges are needed for a specific patch and verify if user have them.

the realistic way of doing this would be to add a notice before the run that says that all SQL patches are run as a regular user ORRR to use the quat credentials when/if they are available ?

I implemented the "very Ideal way" 😸

im very ideally impressed

ridz1208 · 2018-10-02T14:00:24Z

tools/update.php

+}
+// Only apply SQL patches if user explicitly requests it
+$apply_patches = false;
+if (in_array('--apply-patches', $argv)) {


always better to use true as 3rd arg when using in_array()

see http://php.net/manual/en/function.in-array.php#106319

same for all below

ridz1208 · 2018-10-02T14:03:16Z

tools/update.php

+if (updateSourceCode($loris_root_dir, $backup_dir)) {
+    echo '[+] LORIS source code files successfully updated.' . PHP_EOL;
+}
+


shouldn't it check the php version, apache version, mysql version before starting ?

for example 20.0.0 requires apache 2.4 if the update runs without prompting for it, everything is gonna look fine but the website will not be functional

Yes this is a good idea.

This has been added

Remove duplicate line. Add case where installed minor version is ahead of latest release version when major versions are equal. Remove unrelated script.

tools/update.php

driusan · 2019-01-28T15:28:14Z

Regarding the "This PR has been heavily refactored so a lot of the conversation is outdated.", can you close it and send a new PR (when the refactoring is done) so that it's clear what's up to date and what's irrelevant?

Co-Authored-By: johnsaigle <4022790+johnsaigle@users.noreply.github.com>

PapillonMcGill

Pending Travis

ridz1208 · 2019-02-01T21:41:56Z

@johnsaigle Can you re-issue it when/if its done to get a clean history ?

closing for now

johnsaigle added Feature PR or issue introducing/requiring at least one new feature [branch] minor labels May 17, 2018

PapillonMcGill reviewed May 22, 2018

View reviewed changes

ZainVirani previously requested changes May 28, 2018

View reviewed changes

xlecours previously requested changes May 30, 2018

View reviewed changes

tools/update.php Outdated Show resolved Hide resolved

tools/update.php Outdated Show resolved Hide resolved

tools/update.php Outdated Show resolved Hide resolved

johnsaigle added the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Jun 11, 2018

johnsaigle removed the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Jun 12, 2018

johnsaigle changed the title ~~[Tools] Automated update script~~ [Tools] Automatic update script Jun 12, 2018

johnsaigle changed the title ~~[Tools] Automatic update script~~ [Tools] Script to automate LORIS update process Jun 12, 2018

maltheism added the Passed Manual Tests PR has undergone proper testing by at least one peer label Jun 18, 2018

johnsaigle added this to the 20.1.0 milestone Jul 4, 2018

PapillonMcGill suggested changes Jul 9, 2018

View reviewed changes

johnsaigle mentioned this pull request Jul 16, 2018

[RELEASE] Version + #3814

Merged

ridz1208 previously requested changes Jul 16, 2018

View reviewed changes

johnsaigle added the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Jul 17, 2018

johnsaigle added the Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix label Aug 27, 2018

johnsaigle removed the Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix label Sep 4, 2018

PapillonMcGill suggested changes Sep 5, 2018

View reviewed changes

PapillonMcGill previously approved these changes Sep 6, 2018

View reviewed changes

driusan removed this from the 20.1.0 milestone Sep 6, 2018

ridz1208 reviewed Oct 2, 2018

View reviewed changes

johnsaigle added 18 commits January 3, 2019 13:31

Remove support for multiple major release upgrade

b4b0bec

Remove dev detection as it doesn't work since 20

51fa532

phan

19f91c3

Remove unrelated script

bc68742

Incorporate Melanie's feedback

dcc56a3

Remove duplicate line. Add case where installed minor version is ahead of latest release version when major versions are equal. Remove unrelated script.

Fix bug where bkp folde rnot properly named

c6b4f8c

Remove outdated comments

4d2a97a

Use true argument for in_array

f7ae574

Check minimum apache version before continuing

95874bc

Add warning for multiple minor version update

8c8e1bc

Check SQL privileges before executing a patch

a49f3d6

Refactor into two files

fe91848

Cleanup (Melanie's feedback)

24b71fd

Rida's comment

6ab1482

Install packages before updating source code

571ceda

Fix error where tool would always apepar installed

f4cd565

Fix bug where DB connection failed

465c6bb

Remove all code unrelated to SQL patches

543919e

johnsaigle changed the title ~~[Tools] Script to automate LORIS update process~~ [Tools] Script to automate LORIS SQL patching Jan 3, 2019

johnsaigle added Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix and removed Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix labels Jan 23, 2019

Cleanup code

7f76b70

PapillonMcGill suggested changes Jan 28, 2019

View reviewed changes

tools/update.php Outdated Show resolved Hide resolved

tools/update.php Outdated Show resolved Hide resolved

PapillonMcGill and others added 2 commits January 28, 2019 10:43

Update tools/update.php

88576f1

Co-Authored-By: johnsaigle <4022790+johnsaigle@users.noreply.github.com>

Update tools/update.php

a48b4c7

Co-Authored-By: johnsaigle <4022790+johnsaigle@users.noreply.github.com>

PapillonMcGill approved these changes Jan 28, 2019

View reviewed changes

ridz1208 closed this Feb 1, 2019

driusan removed this from Review in Roadmap Aug 13, 2019

[Tools] Script to automate LORIS SQL patching #3677

[Tools] Script to automate LORIS SQL patching #3677

Conversation

johnsaigle commented May 17, 2018 • edited Loading

Overview

Details

To Test

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnsaigle Jun 11, 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

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

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

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

johnsaigle commented Aug 6, 2018

johnsaigle commented Sep 4, 2018

Choose a reason for hiding this comment

johnsaigle commented Sep 5, 2018

PapillonMcGill left a comment

Choose a reason for hiding this comment

johnsaigle commented Oct 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ridz1208 Oct 2, 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

driusan commented Jan 28, 2019

PapillonMcGill left a comment

Choose a reason for hiding this comment

ridz1208 commented Feb 1, 2019

johnsaigle commented May 17, 2018 •

edited

Loading

johnsaigle Jun 11, 2018 •

edited

Loading

ridz1208 Oct 2, 2018 •

edited

Loading