Batch migrate: missing error message if target platform doesn't meet dependencies
mig5 - October 2, 2009 - 01:15
| Project: | Hosting |
| Version: | 6.x-0.4-alpha1 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Steps to reproduce:
Add a new platform i.e Drupal 5.20
Click on the main aegir platform's batch Migrate
Choose Drupal-5.20 as target
Validation runs, but the form that is given is blank. It should show a message 'Unmet dependencies prevent this site from being migrated to this platform'. I thought I submitted a patch that included such a thing.
Screenshot attached
| Attachment | Size |
|---|---|
| migrate_2.png | 130.69 KB |
#1
Actually, somehow, the
$batch['form_state']['storage']['failed']'The following sites will not be migrated' doesn't appear to ever occur. If one site passes but another fails, the one that passes is shown. If no sites pass, the blank form is shown.#2
So I just saw in hosting_migrate_platform_batch(), the entire passed/failed logic is wrapped in a conditional that I didn't see before.
<?phpif ($profile->title != 'hostmaster') {
?>
So that explains it, and this appears thus specific to the main aegir site.
Should we not just set this case to 'failed' so it's still shown rather than a blank form? Or perhaps the main aegir site is soon to be able to be migrated (but maybe not through this batch method? ) I am out of touch with what's been happening with that right now :)
#3
I see the 'Following sites will not be migrated' does appear if there are actually other sites on the Aegir platform and they don't meet the dependencies.
So it's just a corner case where there are no sites on the drupal6 platform that the main aegir site also runs on, and a batch migrate is attempted on that platform. Perhaps not a blocker here.
#4
In the attached patch I move the check for whether the profile is hostmaster, down into after the package comparison is made.
The effect this has, is that the hostmaster site is still always a 'fail' here, which is good, and this means it gets included in the list of 'The following sites will not be upgraded', which is better than a blank form entirely when no other sites exist on that platform.
However this means, I guess, that there's slightly more processing brought in, i.e we are comparing the hostmaster site's packages here and it's kind of redundant since we won't be migrating it anyway. For this reason I guess this is not ideal.
I tried to keep the hostmaster profile check where it was and just extend the conditional with an else statement that showed the 'failed' errors, but this seemed to disrupt the behaviour of a regular site where it will fail in a batch migrate check to a lower platform.
#5
Spoke to Adrian about this and have committed.
#6
Automatically closed -- issue fixed for 2 weeks with no activity.