I'm trying to delete an environment in devshop and the "Delete backups" task is failing...

I think it's because there are no "task args" for some reason.

This is preventing it from destroying the site, also blocking any further action on the site.

Patch and branch to be submitted shortly, suggesting changing from "drush_set_error()" to drush_log() with a warning.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jon Pugh created an issue. See original summary.

Jon Pugh’s picture

  • Jon Pugh committed 8be5c9a on 2731471-backup-delete-warning
    Issue #2731471 by Jon Pugh: Provision backup-delete command is too...
Jon Pugh’s picture

Attaching a patch for hosting.module that prevents a warning caused by the missing task_args.

  • Jon Pugh committed 8be5c9a on 7.x-3.x
    Issue #2731471 by Jon Pugh: Provision backup-delete command is too...
gboudrias’s picture

Status: Needs review » Fixed

Thanks. Looks good, branch is merged, please delete.

Jon Pugh’s picture

Done. Thanks!

ergonlogic’s picture

Status: Fixed » Needs work

Wait... When called without args backup-delete ought to fail. Why would it do otherwise? If you don't have any backups to delete, just don't call it.

I think these should be reverted and fixed wherever backup-delete is being called without args.

  • ergonlogic committed 5b722c6 on 7.x-3.x
    Revert "Issue #2731471 by Jon Pugh: Provision backup-delete command is...
ergonlogic’s picture

As per my above comment, I reverted commits 8be5c9a144 (from Provision) and 0f348e0f027 (from Hosting). They serve only to mask the underlying problem. I think we should take the opposite approach, as per the patch attached, and make the argument to backup-delete mandatory, which it always has been, really.

Presumably this is where the DevShop environment is being deleted? Take a look at these, as we now enqueue backups to be deleted on deletion of a site, under some circumstances: hosting_site_form_hosting_task_confirm_form_alter() and hosting_site_post_hosting_backup_delete_task().

ergonlogic’s picture

Status: Needs work » Needs review

There's a patch in #10 that requires review.

helmo’s picture

Status: Needs review » Reviewed & tested by the community

makes sense

  • helmo committed f8545ac on 7.x-3.x authored by ergonlogic
    Issue #2731471 by Jon Pugh, ergonlogic: Provision backup-delete command...
helmo’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Jon Pugh’s picture

Status: Closed (fixed) » Needs review

This wasn't fixed, it was reverted, which is disappointing.

This is not just covering up an underlying problem. This is a part of the problem. If a site installation fails, you should be able to delete it and it's backups, even though there are no files to delete.

When an installation fails, you are stuck with the site. The "Delete site" task will fail, and if you have the setting for it, the "Delete backups" task will fail as well.

helmo’s picture

@Jon Pugh what about this patch. It think that it addresses the root cause here.

helmo’s picture

Project: Provision » Hosting

Moving to the hosting project as the new patch belongs there.

ergonlogic’s picture

Status: Needs review » Reviewed & tested by the community

This looks reasonable.

Jon Pugh’s picture

Yep, that looks like it!

Thanks, Helmo.

  • helmo committed 136337e on 7.x-3.x
    Issue #2731471 by Jon Pugh, ergonlogic, helmo: Skip backup-delete task...
helmo’s picture

Status: Reviewed & tested by the community » Fixed

committed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.