Since I'm using aegir on docker for development, I'm looking at the running hosting-queued output on a regular basis.

It's not informative at all unless you use the -v flag.

This patch moves the drush_log and watchdog calls to the `drush hosting-task` command, and outputs the title and node ID of the task.

Started hosting queue daemon. Waiting for new tasks.                        [ok]
Starting task "Reset password:                                              [ok]
site.drupal.devshop.local.computer" [node/510]
Finished executing task "Reset password:                                    [ok]
site.drupal.devshop.local.computer" [node/510].
CommentFileSizeAuthor
#35 2828630-hosting-queued-logs.patch409 bytesJon Pugh
#26 2828630-hosting-queued-logs.patch3.92 KBJon Pugh
#18 2828630-hosting-queued-logs.patch3.41 KBJon Pugh
#10 2828630-hosting-queued-logs.patch2.69 KBJon Pugh
#8 2828630-hosting-queued-logs.patch2.68 KBJon Pugh
#3 2828630-hosting-queued-logs.patch1.65 KBJon Pugh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jon Pugh created an issue. See original summary.

  • Jon Pugh committed 5481474 on 2828630-hosting-queued-logs 
    Issue #2828630: More verbose drush logs for drush hosting-tasks command
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
FileSize
2828630-hosting-queued-logs.patch1.65 KB
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
Issue summary: View changes

  • Jon Pugh committed 7fcb996 on 2828630-hosting-queued-logs 
    Issue #2828630: More verbose drush logs for drush hosting-tasks command
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Active » Needs review

nice...
One minor thing ...we could change 'Finished executing task' to 'Finished task'. Then start and finish messages are equal in length... and thus align.

  • Jon Pugh committed 5e33629 on 2828630-hosting-queued-logs 
    Issue #2828630: More verbose drush logs for drush hosting-tasks command...
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
FileSize
2828630-hosting-queued-logs.patch2.68 KB

Great idea. I also added the same messages to drush hosting-tasks command. This runs separately than the drush hosting-queued command.

I tried putting these messages into the drush hosting-task command, but any logs there are saved into hostmaster. The point of this patch is to improve the output of the queue command.

Branch pushed and new patch attached.

  • Jon Pugh committed 93f10ee on 2828630-hosting-queued-logs 
    Issue #2828630: Fixing bad call to dt();
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
FileSize
2828630-hosting-queued-logs.patch2.69 KB
ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
CreditAttribution: ergonlogic at Ergon Logic Enterprises for Aegir Cooperative commented

In the interest of DRY, could we move this logging to common functions? Something like hosting_task_log_start($task) and hosting_task_log_finish($task)?

  • helmo committed f959691 on 2828630-hosting-queued-logs 
    Issue #2828630 by Jon Pugh, helmo: Make the log format more consistent...

  • helmo committed fb0e33a on 2828630-hosting-queued-logs 
    Issue #2828630 by Jon Pugh, helmo: move this logging to common functions
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented

I added one commit to the feature branch to Make the log format more consistent ... trailing dots for all.
And moved this logging to common functions as #11suggested.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
Status: Needs review » Needs work

I disagree with adding two new functions that will never be implemented again.

We already have too many functions like that in this project.

Most of the command drush hosting-queued is a copy of drush hosting-tasks.

If you are interested in DRY, I would start there. the hosting-tasks command could just take an option to run as a daemon, couldn't it?

  • Jon Pugh committed 112614b on 2828630-hosting-queued-logs 
    Issue #2828630: Improve hosting-tasks and hosting-queued logging by...

  • Jon Pugh committed 93a8b48 on 2828630-hosting-queued-logs 
    Issue #2828630: Add a new function to execute a hosting task:...
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
Status: Needs work » Needs review
FileSize
2828630-hosting-queued-logs.patch3.41 KB

New commits:

  • Remove the "start" and "finish" functions and add a new function: hosting_task_execute($task, $fork = TRUE).
  • Output the number of tasks found, or a message if no tasks were found.

  • Jon Pugh committed 97b2afc on 2828630-hosting-queued-logs 
    Issue #2828630: Add duration to finished task message, and if task is...
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Needs review » Needs work

I tested it again on my quickreview branch and found two problems. I squash merged it into https://github.com/aegir-project/hosting/pull/8 to keep the history readable.

1. Even though I had started the hosting queued without a verbose flag it started printing verbose things like 'Platforms path /var/aegir/platforms exists.' after merging this.

2. Duration is not set

Undefined variable: backend_options hosting_task.module:765                    [notice]
Undefined property: stdClass::$duration hosting_task.module:778                [notice]
Finished task "Verify Site: s7.dev.example.com" with status "Successful" in 0 sec [node/8570].

  • Jon Pugh committed 5ee3310 on 2828630-hosting-queued-logs 
    Issue #2828630: Add $backend_options as the function argument so hosting...
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
Status: Needs work » Needs review

Sorry about that, I committed this to the wrong branch and then never pushed!

Updated branch and patch makes $backend_options an argument so hosting-queued can use interactive and hosting-tasks can use fork.

helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented

That's better on the verbosity :)

The '0 sec' is still there though... the frontend has 10 seconds recorded. If the value is just not available then we could just not show it.

  • Jon Pugh committed f40d723 on 2828630-hosting-queued-logs 
    Issue #2828630: Use "delta" not "duration". "duration" is from DevShop.
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented

Ah, my mistake, "duration" is a property devshop adds that is already formatted by format_interval().

Changed to "delta".

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
FileSize
2828630-hosting-queued-logs.patch3.92 KB
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Needs review » Reviewed & tested by the community

:)

  • helmo committed afb357b on 7.x-3.x authored by Jon Pugh 
    Issue #2828630: More verbose drush logs for drush hosting-tasks command
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
Status: Closed (fixed) » Active

One more change is needed...

drush @hostmaster hosting-tasks should be able to be run in non-forked processes. This way we know the tasks are complete when the command ends.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented

The reason is we turn off the queue for test runs. A behat step exists for running "drush hosting-tasks", which should not fork processes so we can skip the "and I wait 10 seconds" step.

  • Jon Pugh committed a40e75d on 2828630-hosting-queued-logs 
    Issue #2828630: Add a drush option to allow optionally not forking task...
  • helmo committed afb357b on 2828630-hosting-queued-logs authored by Jon Pugh
    Issue #2828630: More verbose drush logs for drush hosting-tasks command
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented

New commits to the branch and patch attached.

All this patch does is allow for forking to be blocked by passing a '--fork=0' option.

This is really only for testing purposes. Blocking forking also prevents task logs from being saved in the front-end. But for running tests, we want the command to run and output to terminal, so this is really helpful. See the test result logs here: https://travis-ci.org/opendevshop/devshop/jobs/188910448#L2657

When I run drush "hosting-tasks --fork=0 --strict=0"
Then print last drush output

This is actually really important for good testing. We don't want the test to continue until the hosting-tasks run is complete. Right now we added a "Then I wait X seconds" step to mitigate this.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location New Windsor NY
CreditAttribution: Jon Pugh at OpenDevShop Inc (CLOSED) commented
FileSize
2828630-hosting-queued-logs.patch409 bytes
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Active » Reviewed & tested by the community

sounds good

  • helmo committed 4769237 on 7.x-3.x authored by Jon Pugh 
    Issue #2828630 by Jon Pugh, helmo: Make forking optional
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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