tollmanz / wordpress-pecl-memcached-object-cache

A WordPress object cache that uses the memcached (not memcache) PECL extension.
233 stars 114 forks source link

Allow settings via Environment variables #57

Closed gmazzap closed 9 years ago

gmazzap commented 9 years ago

First of all thanks for your work this.

Me, @franz-josef-kaiser and other fellows are very interested on the usage of this package on a pretty big website and integrate it in our workflow that is based on Composer and environment variables.

I already seen there's a PR about supporting Composer, and that's fine, so what is missing is the second part.

What I have in mind is something that affect only WP_Object_Cache constructor, that should be something like this:

public function __construct( $persistent_id = NULL ) {

    global $memcached_servers;

    // some code here

   $default_servers = array( array( '127.0.0.1', 11211 ) );

   if ( ! isset($memcached_servers) && ( $env = getenv( 'WP_MEMCACHED_SERVERS' ) ) ) {
     $servers = explode( ';', $env );
     $memcached_servers = array();
     foreach( $servers as $server ) {
          $data = explode( ':',  $server );
          if ( ! empty( $data ) ) {
            $port = isset( $data[1] ) && is_numeric( $data[1] ) ? (int) $data[1] : 11211;
            $memcached_servers[] = array( $data[0], $port );
          }               
       }
   }
   $this->servers =  $memcached_servers ? $memcached_servers : $default_servers;

   // rest of the code here
}

In this way would be pretty simple to add servers like this:

putenv( "WP_MEMCACHED_SERVERS=127.0.0.1:11211;127.0.0.2:11211" );

With a little edit on code above it would be also possible support unix socket (as proposed in #42) and for authentication (related: #32):

// unix socket
putenv( "WP_MEMCACHED_SERVERS=unix:/tmp/memcached.sock" );

// authentication
putenv( "WP_MEMCACHED_SERVERS=username:password@127.0.0.1:11211" );

Finally, a very similar approach can be used to support various Memcached settings (related: #40) using a single ENV var (e.g. WP_MEMCACHED_CONFIG) or more variables, one per setting.

I think that a configuration based on environment variables would be apprecciated by a lot people: Bedrock is pretty popular, and WP Starter is young but promising, even if I'm biased regarding the latter.

Moreover, per code above, it only has effect if global var is not set and the environment variable is, so it will not affect who don't use environment vars.

If you like this idea I can make a PR.

franz-josef-kaiser commented 9 years ago

Subscribing myself to this issue. Btw, @Giuseppe-Mazzapica … I like your version of my name. I should be more YOsef these days.

gmazzap commented 9 years ago

@franz-josef-kaiser LOL (fixed)

tollmanz commented 9 years ago

Sounds reasonable to me. I'd happily review a PR.

Since this is something it sounds like you guys do regularly, do you have any other projects that take a similar approach?

I already seen there's a PR about supporting Composer, and that's fine, so what is missing is the second part.

Happy to pick this up as well. I don't use Composer with WordPress frequently, so happy to take some direction from those who do.

gmazzap commented 9 years ago

Sounds reasonable to me. I'd happily review a PR.

Perfect, I'll work on it very soon.

Since this is something it sounds like you guys do regularly, do you have any other projects that take a similar approach?

Usage of environment variables in WordPress is something that I started to do recently. Before that, I used them in Laravel and general PHP works. I started to use them in WordPress since I released WP Starter and since then I've used env vars in different works, but unluckily nothing public. Roots (and its components) are pretty popular packages that make use of env vars.

tollmanz commented 9 years ago

Closing this as @Giuseppe-Mazzapica opened the more actionable #58.

tollmanz commented 9 years ago

I landed Composer support in #51. I know you guys use Composer more extensively with WP. If you have any suggestions, please let me know.