The docs for hook_update_N() and update_do_one() explain that you should throw DrupalUpdateException when an update fails, to prevent the schema version from being changed.

Trying to test this out with drush, I thought I was going crazy, until I realized that it just wasn't printing anything.

CommentFileSizeAuthor
#2 drush-1380948-2.patch2.04 KBtim.plunkett

Comments

moshe weitzman’s picture

moshe weitzman commented

Would love some help figuring out what drush should do here. Don't think the maintainers are going to give this much attention before a patch arises.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Title: Print warning from thrown DrupalUpdateException » Print errors from caught exceptions during updatedb
Category: feature » bug
Status: Active » Needs review
StatusFileSize
new drush-1380948-2.patch2.04 KB

So it turns out that exceptions are already being caught, and there is no need to catch a more specific one.

However, they're not printing out.

Reading through #1434376: Errors not propagated, make silently "succeeds" but doesn't do anything and similar, I think it's just a matter of setting $integrate to TRUE. And I also cleaned up the message to not claim that it is performing an update that it knows just failed.

tizzo’s picture

tizzo commented
Status: Needs review » Needs work

This patch is causing craziness to be printed at the end of each batch section in the operation if $context['message'] is used.

Example:

A redirect already exists from club/northview-middle-school to webform-submission/721490.                                                                                                                                                                            [ok]
A redirect already exists from club/princeton-pack to webform-submission/721491.                                                                                                                                                                                     [ok]
A redirect already exists from club/boston-community to webform-submission/721492.                                                                                                                                                                                   [ok]
log"}

[…sixteen blank line returns…]

DRUSH_BACKEND:{"type":"ok","message":"A redirect already exists from club\/villages-charter-school to webform-submission\/721493.","timestamp":1334893727.4381,"memory":53461560,"error":null,"pacA redirect already exists from club/mti-college-sacramento to webform-submission/721494.                                                                                                                                                                             [ok]    
Progress = 6611 out of 6611 for migration jsonclub (2/2)                                                                                                                                                                                                             [ok]
ket":"log"}

The `log"}` through DRUSH_BACKED:{` sections should not be printed (especially with the big hunk of whitespace).

steven jones’s picture

steven jones commented
Version: » 8.x-6.x-dev

@tizzo we've seen similar DRUSH_BACKEND issues, and have raised an issue with some thoughts here: #1829418: Drush invoke process can't handle large packets of data

moshe weitzman’s picture

moshe weitzman commented
Assigned: Unassigned » greg.1.anderson

Don't know what to do with this report. Assigning to Greg. He might choose to close it.

greg.1.anderson’s picture

greg.1.anderson commented

Likely related to #1982502: Reduce complexity of backend invoke; consider removing concurrency.

moshe weitzman’s picture

moshe weitzman commented

We fixed #1982502: Reduce complexity of backend invoke; consider removing concurrency which would explain some "craziness" that tizzo was seeing. We should probably evaluate this patch again.

moshe weitzman’s picture

moshe weitzman commented

If noone updates this issue with more current info, I'll close it.

greg.1.anderson’s picture

greg.1.anderson commented
Assigned: greg.1.anderson » Unassigned
Status: Needs work » Needs review

I think that #3 was fixed in other issues, but #2 still needs review. Anyone have any good test data to try this with? Unassigning.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

#2 looks promising.We need some steps to reproduce in order to verify the fix.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Needs work » Closed (won't fix)
Issue tags: +Needs migration

This issue was marked closed (won't fix) because Drush has moved to Github.

If desired, you may copy this bug to our Github project and then post a link here to the new issue. Please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.