On Tue, Jan 3, 2012 at 1:08 PM, Andrew Petro <ape...@unicon.net> wrote:
> Scott, > > Is your position that adding new required properties to cas.properties for > the CAS 3.5 release is unacceptable? > My position is (a) don't make people do things that aren't necessary (if we tell people to modify cas.properties, and we have NEW defaults, put it in a defaults file so its one less thing for them to worry about during upgrade, one less question for the list, etc.) and (b) putting it in cas.properties, which requires effort on the part of the deployers, limits WHERE and WHEN you can add new defaults (i.e. you could ONLY do it in a 3.4 -> 3.5 style upgrade vs 3.4.11 -> 3.4.12). I've made no comment about NEW required properties. We haven't had any new properties lately that didn't just need a sensible default that I'm aware of. Hope that is more clear. Cheers, Scott > > (Such that adding a mechanism like the one Daniel Frett proposes is > required for its effect of making the new required cas.properties > properties not actually required by instituting fallback defaults.) > > Or do you agree with me that, in principle, adding new required > cas.properties properties is an acceptable burden to place on upgraders in > the course of upgrading to a new minor CAS release, e.g. from 3.4.x to 3.5? > > I'm trying to understand your perspective. Certainly, if the only way to > accommodate evolutionary change to the set of cas.properties properties is > to implement a scheme for making the properties optional, then being able > to add cas.properties properties is more valuable than is avoiding the > proposed complexity. If I believed that not having this default.properties > file mechanism blocked progress on evolving cas.properties, than I'd want > default.properties. Is this the perspective you're coming from? > > > My perspective is that requiring additional cas.properties properties (and > retiring previously valid such properties) is an acceptable burden to place > on CAS upgraders across minor upgrades, as in from 3.4.x to 3.5, and that > as such one option available to the CAS server product is to continue with > the present approach to placeholders in the Spring configuration and > cas.properties properties, updating the set of properties and their default > values for the CAS 3.5 release, and expecting upgraders to update their > local cas.properties configuration accordingly. And, my perspective is > that, among the available options, this simpler status quo option is > slightly better than is the multiple overlapping properties files option. > > Kind regards, > > Andrew > > > On Jan 3, 2012, at 11:05 AM, Scott Battaglia wrote: > > >> > >> On Tue, Jan 3, 2012 at 10:59 AM, Andrew Petro <ape...@unicon.net> > wrote: > >> 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. > >> > > Minimizing upgrade pain and placing the defaults in a single location > when we realize something should be paramaterized is not worth solving? > I'm confused. Especially since not having a mechanism like this in place > couldn't add parameters except in major releases or if we spread the > defaults throughout all the files (a la the Spring 3 syntax) > > > > > > > >> > >> > >> > >> > >> 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: arch...@mail-archive.com To unsubscribe, change settings or access archives, see http://www.ja-sig.org/wiki/display/JSG/cas-dev
