For some reason (probably obvious), I still can't fetch the Nginx version check results in vhost.tpl.php while I can in server.tpl.php.

I suppose it is related to all this "context" stuff, so I'm probably missing something simple here.

Anyway, we have a check in nginx_service.inc and it works, since correct values are written in server_master.alias.drushrc.php:

  'nginx_has_gzip' => 1,
  'nginx_has_upload_progress' => 1,
  'nginx_has_new_version' => 1,

I can then fetch that in server.tpl.php with something like:

$nginx_has_gzip = drush_get_option('nginx_has_gzip');
if ($nginx_has_gzip) {
   print "  gzip_static       on;\n";
}
$nginx_has_upload_progress = drush_get_option('nginx_has_upload_progress');
if ($nginx_has_upload_progress) {
   print "  upload_progress uploads 1m;\n";
}

because it is the same (server) context ( I guess).

Now the question is: how to fetch those values in vhost.tpl.php, since this doesn't work:

  if ($server->nginx_has_new_version || $server->nginx_has_upload_progress) {
    print "   include      " . $server->include_path . "/nginx_advanced_include.conf;\n";
  }
  else {
    print "   include      " . $server->include_path . "/nginx_simple_include.conf;\n";
  }

It is always using "nginx_simple_include.conf", so it looks like $server->nginx_has_new_version is not available here.

Ideas?

CommentFileSizeAuthor
#7 nginx_provision_register_962188.patch690 bytescrea

Comments

Anonymous’s picture

Should it be $this->server instead of $server? Not sure.

Have you tried print_r($server) to see if that value is actually set?

Anonymous’s picture

Status: Active » Postponed (maintainer needs more info)

Try d()->server->nginx_has_new_version

Let me know if it works.

omega8cc’s picture

It doesn't work. I really don't understand why, while I can use any other values stored in server_master.alias.drushrc.php and it works, for example $server->include_path. I tried already a few combinations and none works in vhost.tpl.php.

crea’s picture

Version: » 6.x-1.1

Subs

crea’s picture

I suspect we must register context properties added by nginx http service (probably using setProperty()).
Then it's a good question why they work at all, if they are unregistered, but that could be a coincidence (another silent bug).

crea’s picture

If you run
drush eval "print_r(d('server_master'));"
there's no added nginx-related context properties anywhere.

crea’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new690 bytes

Seems to make difference

omega8cc’s picture

Status: Needs review » Reviewed & tested by the community

Excellent! It works!

Thank you :)

crea’s picture

Though we still need to understand why the properties work without registering in the server template

omega8cc’s picture

It works in the server template only because we use drush_get_option() there to fetch them, which works in the server context, I think.

crea’s picture

If you look at drush_get_option() code it allows to provide context type, and if none is provided, it tries all context types one by one. So if drush_get_option() worked in the server template, and didn't in the host one - that could be a bug
Also, why initialization of server context in the host template is different from the server template ? This is inconsistent, and doesn't make sense IMO.

anarcat’s picture

Status: Reviewed & tested by the community » Patch (to be ported)
+++ http/nginx/nginx_service.incundefined
@@ -15,6 +15,11 @@ class provisionService_http_nginx extends provisionService_http_public {
+    ¶

there is extra whitespace here, but i'll commit this anyways. :)

otherwise this patch looks sound... the reason why they work within the server context is that they are used when the server is being verified, so the drush options are actually set, by the frontend in the verify task. when you verify the site, the frontend doesn't pass the server data anymore, it assumes it's already available in the backend. by using "setProperty", you ensure that this data persists in the backend and is usable from other places than the server verification.

This has been committed to 2.x and seems harmless enough that I merged it in 1.x too.

Powered by Dreditor.

anarcat’s picture

Status: Patch (to be ported) » Fixed

this has been merged.

Status: Fixed » Closed (fixed)

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

  • Commit 5f2e9a0 on 7.x-2.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x by anarcat:
    #962188 by crea: Fixed Nginx version check works for server.tpl.php but...
  • Commit ddfe352 on 6.x-1.x, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x by anarcat:
    #962188 by crea: Fixed Nginx version check works for server.tpl.php but...