We should provide a separate setting in the admin interface for toggling profiling of Drush (command line) requests on or off.

CommentFileSizeAuthor
#8 XHProf-no-drush-toggle-1803866-8.patch3.04 KBElin Yordanov
#7 XHProf-no-drush-toggle-1803866-7.patch3.11 KBElin Yordanov
#6 XHProf-no-drush-toggle-1803866-6.patch2.49 KBjrbeeman
#2 XHProf-no-drush-toggle-1803866-2.patch2.31 KBjrbeeman
#1 XHProf-no-drush-toggle-1803866-1.patch2.31 KBjrbeeman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jrbeeman’s picture

jrbeeman
he/him
Primary language English
Location Arizona, USA
CreditAttribution: jrbeeman commented
FileSize
XHProf-no-drush-toggle-1803866-1.patch2.31 KB
jrbeeman’s picture

jrbeeman
he/him
Primary language English
Location Arizona, USA
CreditAttribution: jrbeeman commented
FileSize
XHProf-no-drush-toggle-1803866-2.patch2.31 KB

Argh - bad update hook name in first patch. New patch attached.

erikwebb’s picture

erikwebb CreditAttribution: erikwebb commented
Status: Active » Needs review
erikwebb’s picture

erikwebb CreditAttribution: erikwebb commented

Note: This patch will need to change if #1803858: Make "Disable profiling of admin pages" configuration element dependent on enabled setting is accepted first.

jrbeeman’s picture

jrbeeman
he/him
Primary language English
Location Arizona, USA
CreditAttribution: jrbeeman commented

Agreed. I suggest waiting on #1803858: Make "Disable profiling of admin pages" configuration element dependent on enabled setting to be complete before moving on this.

jrbeeman’s picture

jrbeeman
he/him
Primary language English
Location Arizona, USA
CreditAttribution: jrbeeman commented
FileSize
XHProf-no-drush-toggle-1803866-6.patch2.49 KB

Updated patch, re-rolled against latest 7.x-1.x now that #1803858: Make "Disable profiling of admin pages" configuration element dependent on enabled setting is closed.

Elin Yordanov’s picture

Elin Yordanov CreditAttribution: Elin Yordanov commented
Issue summary: View changes
FileSize
XHProf-no-drush-toggle-1803866-7.patch3.11 KB

I've re-rolled the patch to the current dev version, since it was not applying.
Please someone review.

Elin Yordanov’s picture

Elin Yordanov CreditAttribution: Elin Yordanov commented
FileSize
XHProf-no-drush-toggle-1803866-8.patch3.04 KB

Sorry for the previous patch, my IDE has messed up somethings before creating the patch.
Attached is the new one. Please review.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs review » Needs work 
+++ b/xhprof.install
@@ -0,0 +1,15 @@
+function xhprof_update_7100() {

needs reroll after #2892127: Configuration not deleted on uninstall

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Version: 7.x-1.x-dev » 8.x-1.x-dev
Sharique’s picture

Sharique CreditAttribution: Sharique as a volunteer and at Acquia commented

+1 for separate config for Drush.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

After https://github.com/drush-ops/drush/pull/4705 drush can work without hacks
Also https://github.com/drush-ops/drush/pull/4710 make storage configurable

If both will be commited this one could be closed