Hi again,
On Wed, Mar 12, 2014 at 1:30 PM, Yasuo Ohgaki <[email protected]> wrote:
> Hi Andrey,
>
> On Wed, Mar 12, 2014 at 8:04 PM, Andrey Andreev <[email protected]> wrote:
>>
>> On Wed, Mar 12, 2014 at 2:19 AM, Yasuo Ohgaki <[email protected]> wrote:
>> > Hi Andrey,
>> >
>> > On Wed, Mar 12, 2014 at 8:54 AM, Andrey Andreev <[email protected]>
>> > wrote:
>> >>
>> >> > Users want to have contorl *when* gc is performed rather than luck.
>> >> > This
>> >> > is
>> >> > reasonable. IMO.
>> >>
>> >> I like control, it's nice and *I already have it*:
>> >>
>> >> ini_set('session.gc_probability', 1);
>> >> ini_set('session.gc_divisor', 1);
>> >
>> >
>> > Of course you can.
>> > It does not look nice as API. API should have good name for it. IMO.
>> >
>> > session_gc() is useful for both low traffic site and high traffic site.
>> > I don't see reason not to have explicit gc API. User may simply call
>> >
>> > session_gc();
>> >
>> > periodically by cron, etc.
>>
>> There's a very good reason not to have it, you just ignored most of my
>> previous email which explained exactly that.
>
>
> Users may abuse INI or API.
> If you worry about abuse, it would be better to have API and warn users.
I don't think you understand what my point is ... the API itself must
be hard to abuse.
The INI settings are not that often misused because they are hard to
find for anybody that's not specifically looking for them. This is not
the case for functions and exposing the GC as a function is something
that would at least triple the abuse.
> Users should purge deleted session data for security reasons. It is
> especially important for low traffic sites. High traffic sites may need
> periodic purge due varying request rate.
I don't disagree with that, but it's not what we're discussing.
> Not having GC API is API design bug.
There is an API, you're introducing a second one.
> If there should not be convenient API, file_get/put_contents() aren't needed
> neither.
Nonsense. This is not even worth explaining why.
>> >> >> And while session_serializer_name() is just redundant, session_gc()
>> >> >> could cause performance issues.
>> >> >
>> >> > There is session_module_name(), why not session_serializer_name()?
>> >>
>> >> Why not session_cookie_lifetime()?
>> >> Why not session_cache_limiter()?
>> >> Why not session_entropy_file()?
>> >> etc.
>> >
>> >
>> > Those aren't changed by programs often.
>>
>> Oh, trust me - cookie lifetime is changed way more often than the
>> serializer.
>
>
> It's bad practice, indeed.
> User should not change default 0 (session cookie - i.e. cookie only lives in
> browser's memory and destroyed by browser termination)
>
> If users need auto login or like, they should implement it properly by using
> setcookie(). (i.e. Never use session ID cookie for long life session)
I was just giving you an example of how your assumption is wrong,
auto-login is not the subject of this discussion.
>> And how do you define often anyway? Sessions are something that are
>> setup once, it's rarely the case that a programmer needs to change any
>> of the session INI settings more than once in the same program.
>>
>> > As I wrote, 3rd party save handler implements their own serializers.
>> > Not like above example, it is likely called with session_module_name().
>> > e.g.
>> >
>> > session_module_name('memcache');
>> > session_serializer_name('igbinary');
>>
>> It's your opinion that this is "likely" done that way. I for one have
>> *never* used session_module_name().
>> And again, this is not something that people would write more often
>> than once in the whole application.
>
>
> It does not matter much if you use often or not.
> As an API design, not having session_serializer_name() is strange.
You just changed your argumentation from "it's often needed" to "it
doesn't matter if it's often used, but not having it is strange".
Again, my stance is that the existance session_module_name() is
strange, not the other way around.
>> > It looks better than
>> >
>> > session_module_name('memcached');
>> > ini_set('session.serializer_handler', 'igbinary');
>>
>> It doesn't look better than:
>>
>> ini_set('session.save_handler', 'memcached');
>> ini_set('session.serialize_handler', 'igbinary');
>
>
> I don't mind having discussion deprecating all API that modifying its INI.
> I'm some where between +1 and 0 for this.
>
>> IMO, this is more explicit and more understandable for people who
>> haven't read the manuals for the "custom" functions.
>>
>> > It's reasonable to have session_serializer_name(), since it would be
>> > used
>> > with session_module_name(). API should have good name here, too.
>>
>> I'm again questioning whether session_module_name() is
>> useful/necessary in the first place.
>
>
> Now I think I understand your point of view.
>
>> > We may discuss what we should do with module functions that modify it's
>> > INI.
>> > There are number of them already. I think you need RFC for this.
>>
>> On this, I agree.
>
>
> Consistency matters. I'm trying to be consistent with current way.
>
> I'm tends to agree deprecating all INI modifying APIs ;)
> Let's have the RFC, I'll vote +1 for it.
So ... you'd vote +1 for deprecating all similar functions, yet you
just added another one without even discussing it? :)
That would be a nice RFC indeed, but I still stand behind my initial
suggestion - both of these functions should be excluded from the 5.6
release.
Cheers,
Andrey.