The _drush_print_log() function was removed after the Drush 8.0.0 release.
We used it in task.hosting.inc

So we need to update our usage of it.

Jenkins noticed the failure: http://ci.aegirproject.org/job/P_Aegir_Puppet_Module_functional_test_Aeg...

CommentFileSizeAuthor
#9 drush_master_changed-2625168-9.patch605 byteshelmo

Comments

helmo created an issue. See original summary.

helmo’s picture

helmo commented

One option is to switch to using _drush_log() but that's not in Drush 6.

Another would be to change it to drush_log() which takes a string instead of an array. But the signature for that function also changed after 8.0.0.

As I'm not a huge fan of using private functions (underscore prefix) I prefer to switch to using drush_log().

But it seems that if we want to keep supporting Drush master we have to have to build in a version switch in our code.

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
commented

I agree that we shouldn't be calling Drush's private functions directly. A version switch appears reasonable here. Let's put a deprecation note too, so that we can drop the version switch when we drop support for older versions of Drush.

helmo’s picture

helmo commented

The reason we used the private function was that we wanted to intercept the log callback. Probably to get access to the timestamp ans such.

Nasty that Drush switched to PSR 7 logging in a minor release :( https://github.com/drush-ops/drush/commit/3657fa99a9c47e4684786ccac71270...

helmo’s picture

helmo commented
Issue tags: +Aegir 3.4
ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
commented

I'm not able to replicate this is normal Aegir operations...

AFAICT, calls to _drush_print_log() only appear in _hosting_task_log(). That function, in turn, is registered as a Drush log callback in drush_hosting_task_validate(). So I don't see how we would ever end up entering into the else clause there.

In fact, I've commented these lines out, and everything just continues to work as expected. I've tried running tasks from the backend as well, and see no difference at all.

Can anyone point out how to trigger this error?

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
commented
Status: Active » Fixed

Compare this passing test with the currently failing test. Note that the failing test adds --debug. I can confirm that the running something like drush @hm hosting-task 145 --force --debug will trigger this error.

Anyway, switching to the new Drush\Log\Logger object isn't really a big deal. See: http://cgit.drupalcode.org/hosting/commit/?id=c8d678f3ad05807f21a799e3d1...

helmo’s picture

helmo commented
Status: Fixed » Active

We still intend to support Drush 6 right? Then this needs a check to see if the _drush_print_log() function exists.

I wondered why our Drush 6 Jenkins test was not failing ... it's not running the tasks via the queue daemon, and thus not attempting to add task logs to the hostmaster db.

I'm working on test coverage in #2666926: Test the hosting tasks queue

helmo’s picture

helmo commented
Status: Active » Needs review
StatusFileSize
new drush_master_changed-2625168-9.patch605 bytes

This patch add such a check. I tried it on the vagrant box from Jenkins ... I'm hoping to see the test fail first ... before committing.

ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
commented
Status: Needs review » Reviewed & tested by the community

Looks good.

helmo’s picture

helmo commented
Status: Reviewed & tested by the community » Fixed

Committed

Status: Fixed » Closed (fixed)

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