Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

variable-set not auto-detecting integer types? #378

Closed
bkosborne opened this issue Jan 15, 2014 · 11 comments
Closed

variable-set not auto-detecting integer types? #378

bkosborne opened this issue Jan 15, 2014 · 11 comments

Comments

@bkosborne
Copy link

I get the feeling that this has been reported numerous times before, but I can't find anything in the github issues (just old Drupal.org posts).

When I try setting a variable
drush vset preprocess_css 0
Drush interprets the 0 as a string.

According to https://drupal.org/node/861026, this was fixed by adding auto detection of integers, but it doesn't seem to work? I see the code is there for it though: https://github.com/drush-ops/drush/blob/master/commands/core/variable.drush.inc#L218.

I'm on 6.2.0.

Am I missing something obvious?

@greg-1-anderson
Copy link
Member

Well, the issue you quote was really intended for the variable-get case, but the documentation does pretty much imply that "auto" format should work for variable-set as well. The issue is that "auto" uses "is_integer()" to determine whether the value is an integer or not; with variable-set, since the value came from the command line, it will always be a string. To make this work right with variable-set, we'd have to switch this test to "is_numeric()" instead; however, that would then give the wrong result with variable-get in instances where there are string-type variables that happen to have numeric values.

Today, you must use --format=integer, or maybe drush ev "variable_set('preprocess_css', 0);" is clearer. Maybe we could have --format=lax, that works like "auto" but includes the is_numeric test. Then vget could default to auto, and vset could default to lax. This might do the right thing for most people most of the time. Occasionally, someone might be surprised when they vset a string variable that happens to have all numeric content. I'm on the fence about which resolution is best.

@bkosborne
Copy link
Author

I hadn't thought about the case where you'd actually want to store a string representation of numbers. Agreed that trying to interpret the string numbers as integers could cause additional confusion.

What I'm trying to do is extend the "sync_enable" extension included with drush to work with setting variables as well, so that after an sql-sync from prod to dev, I can set arbitrary system variables, like turning off CSS and JS aggregation.

I'm currently doing it like this:

function drush_sync_variables_post_sql_sync($source = NULL, $destination = NULL) {
  $variables_to_set = drush_get_option_list('variables');
  if (!empty($variables_to_set)) {
    foreach ($variables_to_set as $variable_name => $variable_val) {
      drush_invoke_process($destination, 'variable-set', array($variable_name, $variable_val), array('exact' => TRUE), array('integrate' => TRUE));
    }
  }
}

Since the numbers are actually coming in from the config and not command line, I should have no problem because they are represented as numbers in PHP to begin with. I think my problem is that drush_invoke_process seems to actually execute via command line - so my args go back to a string.

Do you have recommendation on how to get around this? Maybe I can try using drush_invoke_process but invoke php-eval instead?

@weitzman
Copy link
Member

Have you tried the --format option for vset?.

Otherwise, php-eval would work.

@greg-1-anderson
Copy link
Member

The sync_enable hook does not include a facility for setting variables because you can do this in settings.php. I put the following in my dev sites' settings files once, and then I am done:

// Show all errors
$conf['error_level'] = 2;
error_reporting(E_ALL);
ini_set('display_errors', TRUE);
ini_set('display_startup_errors', TRUE);

$conf['cache_backends'][] = 'includes/cache-install.inc';
$conf['cache_default_class'] = 'DrupalFakeCache';
$conf['cache_class_cache_page'] = 'DrupalDatabaseCache';
$conf['preprocess_css'] = 0;

// Redirect emails to the Devel mail log
$conf['mail_system'] = array(
  'default-system' => 'DevelMailLog',
);
$conf['devel_debug_mail_directory'] = dirname(DRUPAL_ROOT) . '/devel-mail-log';

@bkosborne
Copy link
Author

@weitzman - I'd have to predetermine what kind of data I'm getting from the drush alias settings and then set the appropriate format in the drush_invoke_process call - which is certainly possible. Thanks for the tip.

@greg-1-anderson - Thanks - forgot I could do that. I'll abandon my current approach in favor of that.

@btopro
Copy link

btopro commented Jul 7, 2014

May be related but just realized in order to set a field to negative 1 I had to do this:

php -r "print json_encode("-1");" | drush vset --format=json httprl_server_addr - --y

This worked fine but seems needlessly complex. Also, you can't spider this call (like against an @sites for example) unless someone else knows how to do that

@greg-1-anderson
Copy link
Member

@btopro is --format=integer not working for you with drush vset? Are you using Drush 6 or later? If not, please upgrade and try it again.

@btopro
Copy link

btopro commented Jul 7, 2014

just updated to drush 6.3, now I get an error

drush @courses.music004 vset httprl_server_addr --format="string" "-1"
because the dash - is for specifying options drush seems to be thinking that -1 for NEGATIVE ONE is actually an option flag.

Unknown option: --1. Seedrush help variable-setfor available options. To suppress this error, [error] add the option --strict=0.

@greg-1-anderson
Copy link
Member

That's a fairly serious limitation all right. You could use php-eval (ev) instead:

drush @site ev 'variable_set("httprl_server_addr", -1);'

I'm not sure how in the general case we should allow the user to specify argument names that start with a "-". For the specific case of vset, perhaps we could support --value="-1"

@btopro
Copy link

btopro commented Jul 7, 2014

ended up doing drush @sites sqlq "UPDATE variable SET value='i:-1;' WHERE name='httprl_server_addr'" which is less then ideal; didn't think of the evaluate that's a good idea.

What I found odd is that it was evaluating "-1" as -1 as --1 :) It seemed to me that drush vset thing "-1" should have worked but it hopped right over the quotes.

@greg-1-anderson
Copy link
Member

Drush treats -v and --v the same, and then throws away the leading dashes. The quotes are only for the benefit of the shell; they are gone before Drush gets them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants