Remove NodeSquirrel integration

The parent issue suggests a work-around exists. Does that mean Nodesquirrel works when the work-around is applied?

In that case killing Nodesquirrel would presumably also kill the work-around?

There is an example of disabling B&M 7.x.-x-x functionality in this patch (as yet uncommitted).

I have an ulterior motive in posting this here, because I also want people to review that patch. :-)

As the related issue NodeSquirrel status points out, NodeSquirrel has now officially been sunsetted.

NodeSquirrel will stop accepting backups after September 30, 2019.

Backups will no longer be available for download/restore after November 1, 2019.

This is a simple patch to get things started.

It does three things: 1) comments out NodeSquirrel integration. 2) Creates a warning in the update function. 3) Removes NodeSquirrel references from the tests.

Note that the NodeSquirrel tab will remain visible until the menu cache is rebuilt. Rebuilding the menu cache may be @todo for this issue.

I thought about letting the update function disable schedules that use the NodeSquirrel destination, but that seemed overkill, also considering that the Schedules and Settings tabs neatly remind the user that something is amiss.

The patch addresses the original issue. I expect the issue may change now that the NodeSquirrel service is getting binned by its owners, but I am not familiar enough with the how and what of NodeSquirrel, so I decided to not try and change the issue.

I am not sure what you mean by 'update script'. Do you mean that each update should be in its own implementation of hook_update_N()? That is to say, should the code I added in #9 be moved to backup_migrate_update_7308()?

I sort of assumed the last two digits of the function name track the minor version number, although I now see no evidence of this on the API page. The API page does seem to allow for multiple updates within the same function though: "For each change that requires one or more actions to be performed when updating a site, add a new hook_update_N(), which will be called by update.php." (Emphasis mine.)

Thank you for the explanation.

Here is a new patch that puts the warning in its own update hook.

Your patch applies well. I no longer see Nodesquirrel links or references, my destinations no longer contain Nodesquirrel nor can I create Nodesquirrel destinations.

Why did you move the update hook info text from a return value to a drupal_set_message() call? Using a return value seems to be standard?

As for hook_requirements(), I have no opinion on whether we should use it here. What were your thoughts on the issue?

I have the feeling that in the realm of back-ups, when to warn and how to warn is a potentially thorny issue that I know too little about.

I assume you don't want to overload users with messages, because they will start tuning them out. And I assume that if you warn users, you want them to take action based on that warning. Especially with automated backup schedules, the user may assume that they are OK for back-ups. If a destination is cancelled, that means the user is relying on back-ups that are no longer being made.

Looks good to me. Patch and updates apply well, and the NodeSquirrel destination is removed.

I was not using NodeSquirrel, so I don't know what it would be like for someone that had been using it.

CC Site Test

Tested this patch using an existing Drupal 7 site with an existing Nodesquirrel installation. This included a nodesquirrel backup schedule. To test, I also added a backup script that included nodesquirrel as a secondary backup destination.

- Installed a clean copy of the 3.x-dev version of the module
- Ran update script - which fired the 7308 update script
- Applied the patch #22 - patch applied cleanly with no errors
- Ran update script - which fired the 7309 update script - this deleted the Nodesquirrel backup schedule, and updated the backup script using nodesquirrel as a secondary destination
- Patch and update process was completely successful

Searched the module for any references to nodesquirrel and found none in the codebase.

I'll leave this as Needs Review until I can test on at least one or two other installations.

Some notes:

1)

+ drupal_set_message(t('The backup schedule named "%backup" as been deleted.', array('%backup' => $name)));

The word 'as' should be 'has'.

2)

+ // Clear the cache so that the NodeSquirrel plugin is no longer loaded.
+ cache_clear_all('*', 'cache', TRUE);

According to this documentation page, running update.php already clears caches. Why do it here? Is this to make the menu rebuild (which follows after the cache_clear_all() call) work?

I applied the patch and it seems to work.