10:33 a.m.
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.
10:44 a.m.
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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/wildfly-dev
11:36 a.m.
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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
_______________________________________________
wildfly-dev mailing list
wildfly-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/wildfly-dev
--
Brian Stansberry
Principal Software Engineer
JBoss by Red Hat
4127
days inactive
4127
days old
2 comments
3 participants
participants (3)
-
Brian Stansberry -
Darran Lofthouse -
Kabir Khan