I'm ambivalent about the value of this change to introduce an additional default.properties to be parsed prior to cas.properties, which would supersede default.properties' values. It feels like it's adding complexity without solving a problem worth solving.
Again, this is: https://issues.jasig.org/browse/CAS-1084 and https://github.com/Jasig/cas/pull/23 I'm having trouble empathizing with the problem that this seems to be trying to solve. Updating the cas.properties file on a CAS version upgrade seems like a totally reasonable burden on the upgrader. If CAS started configuring TGT timeouts in cas.properties, I wouldn't regard it as too much to ask *upgrading* CAS adopters to notice the new properties in the shipping-in-CAS cas.properties and either add these to their localized cas.properties or delete their local cas.properties and re-fork from the new default. I don't see having to update a local cas.properties that worked with version 3.4 of CAS to provide the properties required by 3.5 of CAS as a problem at all, and I don't value sparing adopters that particular pain on upgrade. This is different from making new deployers set these values when they first deploy CAS, since new deployers when first deploying CAS don't have an existing cas.properties file that would gum up getting the properties and values in the cas.properties shipping in CAS. I would have concern about CAS shipping with properties files that don't work. That's different from CAS shipping with a cas.properties that does work but worrying that some adopters won't use that working cas.properties. In my experience, updating the local cas.properties file on an upgrade to include added properties just hasn't felt anything like a real problem, just a reasonable upgrade practices checklist item. On balance, I'd probably rather have the fail-init-on-unfulfilled-placeholder behavior than the missing-property-is-masked-by-default.properties behavior. Adding default.properties feels like it's adding some complexity. Currently: Q: "Hey, there's this placeholder ${cas.securityContext.casProcessingFilterEntryPoint.loginUrl} in cas-servlet.xml, where's the value for that set?" A: In the properties file set in propertyFileConfigurer.xml, which by default is /WEB-INF/cas.properties . After this change Q: "Hey, there's this placeholder ${cas.securityContext.casProcessingFilterEntryPoint.loginUrl} in cas-servlet.xml, where's the value for that set?" A: Well, it depends. in propertyFileConfigurer.xml, there's a list of properties files, which by default is /WEB-INF/default.properties and /WEB-INF/cas.properties. The last-parsed value wins. So, if this property is in cas.properties, that's where it's set. But if it's not in cas.properties, then it's the value in default.properties. It's not the end of the world, but the latter felt harder to explain, and the former felt simpler. Currently, if I fat finger a property name in a local cas.properties, I notice. Under the proposed change, the fat fingering is masked by a default value in default.properties. Will CAS upgrading deployers be more grateful that we spared them having to update their cas.properties files on upgrades, or will they be more grateful for missing cas.properties properties continuing to fail fast? It's not clear to me that allowing subsets rather than complete sets of properties in cas.properties files is worth losing the fail-fast-on-missing-properties feature. Would deployers rather have just one properties file to worry about, or would they rather have two and understand what it means for properties to be in which and not the other? It's not clear to me that the complexity is worth it. I think I'm -0 for this change, but I don't think it's very important and I'll happily help upgrading adopters to understand the cascading-properties-files approach if CAS implements it. Andrew On Jan 3, 2012, at 8:18 AM, Scott Battaglia wrote: > I'm not sure I agree with forcing you to add something. If we've moved a > formerly hard-coded property to now being configurable, you shouldn't have to > do anything. If its a new value, we should have a sensible default without > requiring you to choose one (we don't make people set the TGT timeouts when > they first deploy CAS). > > > On Tue, Jan 3, 2012 at 8:14 AM, Marvin Addison <marvin.addi...@gmail.com> > wrote: > I'm really ambivalent about this approach. On the one hand it may > ease the burden of upgrades when new properties are inevitably added. > On the other hand it may facilitate upgrades inheriting undesirable > behavior by default. I personally find it valuable for a deploy to > break due to a new property missing from out custom cas.properties > file, which forces me to review the change and consider whether the > default is in fact desirable. > > M > > -- > You are currently subscribed to cas-dev@lists.jasig.org as: > scott.battag...@gmail.com > To unsubscribe, change settings or access archives, see > http://www.ja-sig.org/wiki/display/JSG/cas-dev > > -- > You are currently subscribed to cas-dev@lists.jasig.org as: ape...@unicon.net > To unsubscribe, change settings or access archives, see > http://www.ja-sig.org/wiki/display/JSG/cas-dev -- You are currently subscribed to cas-dev@lists.jasig.org as: arch...@mail-archive.com To unsubscribe, change settings or access archives, see http://www.ja-sig.org/wiki/display/JSG/cas-dev
