In modules/hosting/task.hosting.inc, function drush_hosting_task() hard codes which Task Types must call provision-save before running the primary task. Only task types "install", "verify", and "import" do this. I have a task in devshop_projects.module that creates a new "context" type in Aegir, but it is blocked by this hard coding.

Current Code From: drush_hosting_task(), modules/hosting/task.hosting.inc:

// On install/verify, save the named context
  if ($task->task_type === 'install' || $task->task_type === 'verify' || $task->task_type === 'import') {
    //...
    $output = drush_invoke_process('@none', 'provision-save', array('@' . $task->ref->hosting_name), $task->context_options, array('method' => $mode));
  }

This patch changes the above to:

// On install/verify, save the named context
  if (!empty($task->task_info['provision_save'])) {
    //...
    $output = drush_invoke_process('@none', 'provision-save', array('@' . $task->ref->hosting_name), $task->context_options, array('method' => $mode));
  }

The following patch changes the logic behind determining what tasks to call provision-save for. Instead of a hard coded fixed list, hook_hosting_tasks() is used to set a $task->provision_save boolean whether or not the task requires this.

Patched version of 'install' task from hosting_site_hosting_tasks():


$tasks['site']['install'] = array(
    'title' => t('Install'),
    'description' => t('Install a site'),
    'hidden' => TRUE,
    'provision_save' => TRUE,
  );

I'd love some feedback to see if this is the right direction, then I will patch against 1.x and 2.x

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jon Pugh’s picture

Issue summary: View changes

adding information

Jon Pugh’s picture

Patch Attached
ROLLED AGAINST 1.9. I need this for DevShop... once I can confirm from an aegir core dev this is the right direction we will target 2.x and 1.x

Jon Pugh’s picture

Status: Active » Needs review
Jon Pugh’s picture

Issue summary: View changes

markup cleaning

Jon Pugh’s picture

Version: 6.x-1.9 » 6.x-2.x-dev
Priority: Normal » Major

This patch applied cleanly to the 2.x branch as well!

Would love some review...

Steven Jones’s picture

Status: Needs review » Needs work

Thanks for the hard work and the patch, we really should get this in 2.x, and it could possibly get into 1.x roo really, as the change is internal.

The patch looks great, a few little whitespace bits in there, but nothing too major.

This needs some documentation adding to the hook_hosting_tasks() function, so that we know we can use this flag!

Jon Pugh’s picture

No problem! I'm working on this now... Thanks so much for the review!

Jon Pugh’s picture

Status: Needs work » Needs review
FileSize
6.39 KB

New Patch attached!

- Whitespace Fixed
- Docs for hook_hosting_TASK_OBJECT_context_options() updated.
- Docs for hook_hosting_tasks() updated.
- Added @see links to important hooks. in drush_hosting_task_validate()
- Added a str_replace('-', '_', $task->ref->type) for the invocation of hook_hosting_TASK_OBJECT_context_options the that I found. I know this is for context object types, but my feeling is, if you are looking for a function, it should always be converted to underscores.
- Cleaned up comments in drush_hosting_task and I added empty lines above some comments for readability. This is a very tricky piece of code and I thought it might help.

Let me know if I went to far with these additional changes!

Thanks for the review!

Steven Jones’s picture

Version: 6.x-2.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

Thanks so much for this patch. Seems to be working really well, so I've pushed it plus some minor tweaks to 6.x-2.x.

We don't need to convert '-' to '_' in the task reference type, because those types are node types, which aren't allowed to contain '-' anyway!

We should get these commits backported to 1.x, which should be easy.

Steven Jones’s picture

Status: Patch (to be ported) » Fixed

And now pushed this into 6.x-1.x. Thanks so much for the work!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

adding "after" code

  • Commit f69f40d on 6.x-2.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-588728-views-integration, dev-1403208-new_roles, dev-helmo-3.x by Steven Jones:
    Issue #1760962 by Jon Pugh: Added Allow any hosting task to flag itself...

  • Commit f69f40d on 6.x-2.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-588728-views-integration, dev-1403208-new_roles, dev-helmo-3.x by Steven Jones:
    Issue #1760962 by Jon Pugh: Added Allow any hosting task to flag itself...