$_ENV vs $_SERVER in LoggerOptionConverter

Hello

Attached patch allowes to set environment variables using
Apaches
SetEnv or mod_rewrite or PHPs putenv() that can be used in
properties files.

I use this to switch the application path between devel,
beta, release
depending on the virtual host.

bye,

-christian-



diff -Nurb log4php/helpers/LoggerOptionConverter.php
log4php.ch/helpers/LoggerOptionConverter.php
--- log4php/helpers/LoggerOptionConverter.php   2007-07-10
14:28:21.000000000 +0200
+++ log4php.ch/helpers/LoggerOptionConverter.php       
2007-07-18 16:50:20.000000000 +0200
 -75,8
+75,8 
 
         if (defined($key)) {
             return (string)constant($key);
-        } elseif (isset($_ENV[$key])) {
-            return (string)$_ENV[$key];
+        } elseif (isset($_SERVER[$key])) {
+            return (string)$_SERVER[$key];
         } else {
             return $def;
         }

Christian Hammers wrote:
>          if (defined($key)) {
>              return (string)constant($key);
> -        } elseif (isset($_ENV[$key])) {
> -            return (string)$_ENV[$key];
> +        } elseif (isset($_SERVER[$key])) {
> +            return (string)$_SERVER[$key];
>          } else {
>              return $def;
>          }
>   
Hi,

I think it's safe to add the $_SERVER variable here, and
check it before 
$_ENV. But still keep $_ENV like this:

         if (defined($key)) {
             return (string)constant($key);
+        } elseif (isset($_SERVER[$key])) {
+            return (string)$_SERVER[$key];
         } elseif (isset($_ENV[$key])) {
             return (string)$_ENV[$key];
         } else {
             return $def;
         }

Do you agree?

Regards,
Knut Urdalen

Knut Urdalen wrote:
> Christian Hammers wrote:
>> ...
>>   
> Hi,
>
> I think it's safe to add the $_SERVER variable here,
and check it 
> before $_ENV. But still keep $_ENV like this:
>
>         if (defined($key)) {
>             return (string)constant($key);
> +        } elseif (isset($_SERVER[$key])) {
> +            return (string)$_SERVER[$key];
>         } elseif (isset($_ENV[$key])) {
>             return (string)$_ENV[$key];
>         } else {
>             return $def;
>         }
>
> Do you agree?
>
> Regards,
> Knut Urdalen
>
>
That looks good too me.

Do we create a jira issue for something like this? so, for
anything that 
would require a commit?
Should every commit require an issue id? (that would be my
assumption)

Alvaro

Alvaro Carrasco wrote:
> Knut Urdalen wrote:
>> Christian Hammers wrote:
>>> ...
>>>   
>> Hi,
>>
>> I think it's safe to add the $_SERVER variable
here, and check it 
>> before $_ENV. But still keep $_ENV like this:
>>
>>         if (defined($key)) {
>>             return (string)constant($key);
>> +        } elseif (isset($_SERVER[$key])) {
>> +            return (string)$_SERVER[$key];
>>         } elseif (isset($_ENV[$key])) {
>>             return (string)$_ENV[$key];
>>         } else {
>>             return $def;
>>         }
>>
>> Do you agree?
>>
>> Regards,
>> Knut Urdalen
>>
>>
> That looks good too me.
>
> Do we create a jira issue for something like this? so,
for anything 
> that would require a commit?
> Should every commit require an issue id? (that would be
my assumption)
>
> Alvaro
>
>
Err... I just saw Curt's message about jira being setup,
which answers 
my questions.

Alvaro

Hello all,

>>>>> "Knut" == Knut Urdalen
<knutphp.no> writes:

    Knut> Christian Hammers wrote:
    >> if (defined($key)) {
    >> return (string)constant($key);
    >> -        } elseif (isset($_ENV[$key])) {
    >> -            return (string)$_ENV[$key];
    >> +        } elseif (isset($_SERVER[$key])) {
    >> +            return (string)$_SERVER[$key];
    >> } else {
    >> return $def;
    >> }
    >> 
    Knut> Hi,

    Knut> I think it's safe to add the $_SERVER variable
here, and
    Knut> check it before $_ENV. But still keep $_ENV
like this:

    Knut>          if (defined($key)) {
    Knut>              return (string)constant($key);
    Knut> +        } elseif (isset($_SERVER[$key])) {
    Knut> +            return (string)$_SERVER[$key];
    Knut>          } elseif (isset($_ENV[$key])) {
    Knut>              return (string)$_ENV[$key];
    Knut>          } else {
    Knut>              return $def;
    Knut>          }

    Knut> Do you agree?

As regards $_ENV superglobal, depending on php.ini
(parameter
variables_order) this variable may or may not be prefilled.
In my
opinion we should not to much on php.ini and therefore
should use
getenv() instead of at least in addition to $_ENV.

Regards,
Michael.

>>>>> "Knut" == Knut Urdalen
<knutphp.no> writes:

    Knut> Michael Schmitz wrote:
    >> As regards $_ENV superglobal, depending on
php.ini (parameter
    >> variables_order) this variable may or may not
be prefilled. In my
    >> opinion we should not to much on php.ini and
therefore should use
    >> getenv() instead of at least in addition to
$_ENV.

    Knut> One thing about getenv(). I remember running
into a
    Knut> performance issue regarding getenv() and
PEAR:ate
    Knut> earlier. My code produced quite a lot of
Date-objects and
    Knut> PEAR:ate
fetching the timezone information each time. So
    Knut> I'm a bit worried that adding getenv() here
could reduce
    Knut> performance. I don't have the time to write a
benchmark test
    Knut> right now, but I just wanted to mention it.

Ok, this might indeed be a problem, but if we call getenv()
only if
neither $_SERVER nor $_ENV contains whatever we're looking
for,
performance should no degrade.