In D7, we have an 'update_log_queries' variable which you can set to TRUE when running updates. If you do, the database query logger is turned on, and the results of the queries are supposed to somehow be displayed at the end of the update process.
Currently, they are not displayed at all.
We may want to wait for #870918: Regression: Multiple messages returned from the same update function aren't displayed to figure out the best way to solve this, since that makes it possible to cleanly print multiple messages to the screen for the same update function (which would be needed for this to work).
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | query-log-870922-2.patch | 1.12 KB | David_Rothstein |
Comments
Comment #1
moshe weitzman commentedI thought we just ditched this feature altogether. IMO, we can ditch it for D7. Programmatic execution of update.php (like drush) can choose to enable a logger if it wants.
Comment #2
David_Rothstein commentedWell, ditching it would certainly be simplest :)
And dumping it on the screen might be overwhelming, since unlike D6 it doesn't just show the queries the module author specifically chooses, but rather every single query that runs, so it might be difficult to figure out how to have update.php present this information nicely.
The attached patch is what it would look like to ditch it. I guess the question is whether there are people who want to run update.php via the core UI and still have the complete logging information available - if they exist they would rather see this optional feature fixed than ditched.
Comment #3
yched commentedIIRC there was some discussion about removing / keeping that feature in #394268: DIE update_sql() DIE!. In the end it was kept, but I don't remember for which intended output. Possibly for a special option in drush updb ?
Comment #4
moshe weitzman commentedA contrib module could add a logger for the update.php and its batch requests. They can figure out how to sensibly display the data.
Comment #5
dries commentedI'd be OK with that but want to wait a bit longer for feedback.
Comment #6
damien tournoud commentedI don't see what harm this does. The integration is pretty nice and clean. I don't see why and how drush would want to hack this feature in, if it's already cleanly integrated here.
Comment #7
sunIf moshe (as Devel maintainer) is happy with this, then I don't see a reason to hold off this patch.
Comment #8
sun#2: query-log-870922-2.patch queued for re-testing.
Comment #9
sunAlthough a good clean-up, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).
Comment #10
David_Rothstein commentedUnless I am missing something, the current code in Drupal 7 does absolutely nothing here.
We should either remove the code that does nothing, or make it do something.
Comment #11
dries commentedCommitted to CVS HEAD. Thanks.