[wildfly-dev] Adding attributes where the default value is different from the legacy implied behaviour

[Brian Stansberry]

I'll probably be inconsistent here, but oh well....

In general I don't like changing default behavior and we should follow 
the approach you outline, Darran.

A new major release provides a bit more flexibility, but we should 
resist the urge to take advantage of that.

I'm fine with the infinispan change in a major release though, because:

1) It doesn't affect application behavior (other than improving perf.)
2) The old behavior is inconsistent with the rest of WF, where 
statistics are off by default. So we have to choose our inconsistency.
3) The odds of this code being backported to EAP 6 or interfering with 
other backports are low. This last one is a bit of insider stuff, but 
it's reality. Diverging the code base from what is in EAP 6 has a cost, 
so the benefit of the change needs to be higher than if there was no 
issue with diverging. This code is already very different from EAP.

So, if the default is changed, what about Kabir's question re: the 
parsers for legacy schemas?

Does using a legacy config document on a new major release imply an 
expectation that previously unconfigurable behavior remains unchanged?

I don't really think so -- after all you're changing a major release and 
all sorts of unconfigurable before and unconfigurable now behavior may 
have changed.

A benefit to doing what Kabir's patch does is you could boot with an old 
config, trigger persistence (which will be in the new schema) and then 
diff the configs to learn what "new settings" are relevant. But that 
"feature" is only good if we are consistent about.

Bottom line: can we be consistent about doing this kind of thing in 
legacy parsers? If we have been, we should continue, if we haven't been 
we shouldn't start.


On 11/13/13 10:44 AM, Darran Lofthouse wrote:
> I had a similar new attribute with different default behaviour that I
> wanted to add to WildFly, I think the only way Brian let it in in the
> end was: -
>    - Define the attribute with the default to match the old behaviour.
>    - Update the config to reflect the desired new behaviour.
>
> In the past I think I updated the parser to assume the attribute was set
> with the old value when parsing older schemas but I think there was
> still a problem with that as it would affect any scripts not setting the
> attribute.
>
> Regards,
> Darran Lofthouse.
>
>
> On 13/11/13 16:33, Kabir Khan wrote:
>> The https://github.com/wildfly/wildfly/pull/5460 (and the linked PR within) below illustrates the ‘problem’. Although Infinispan is where this popped up, this is bound to have happened in other places as well.
>>
>> The latest version of the infinispan subsystem adds a boolean ‘statistics’ which allows us to turn off statistics gathering in the infinispan subsystem. Previously this was always on, i.e. ’true’. We want the user to have to explicitly turn it on, so its default value is ‘false’.
>>
>> However, in the old parsers, it struck me that they do not expose this attribute in their versions of the xsd, and so they before https://github.com/wildfly/wildfly/pull/5460 would have ended up always not setting the attribute, which means ‘false’. This is different behaviour than running the old xml in an older version of AS where this would have resulted in ‘true’, hence this fix.
>>
>>
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> wildfly-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
> _______________________________________________
> wildfly-dev mailing list
> wildfly-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>


-- 
Brian Stansberry
Principal Software Engineer
JBoss by Red Hat