+1
On Wed, Jan 11, 2012 at 3:44 PM, Scott Battaglia
<scott.battag...@gmail.com>wrote:
> I'm -0.9 on this change.
>
> I don't think the proposal actually makes managing or understanding the
> defaults easier.
>
> However, if everyone else feels this is the optimal solution, I won't
> block it with a -1.
>
>
>
> On Wed, Jan 11, 2012 at 3:35 PM, William G. Thompson, Jr. <
> wgt...@gmail.com> wrote:
>
>> I'd like to move forward on this for 3.5 and it seems like we might
>> have come to some consensus. I've restated the proposal below with
>> the addition of robust sample config in cas.properties. Could I get a
>> straw vote (including non-committers) on the following:
>>
>> CAS Configuration Principles
>> * Simple should be easy, complex should be possible
>> * Key config should be easily externalized so that a single war file
>> can easily be deployed to multiple tiers or nodes
>> * Consistent approach
>> * Generally seek to limit the number of files that need to be managed
>> in the overlay to make upgrading easier.
>>
>> Approach
>> * Push all defaults to the bean files where they are defined inline
>> using the Spring 3.x approach and continue to use cas.properties as
>> the deployment override file for simple parameter configuration.
>> (simple should be easy)
>>
>> * Continue to use deployerConfigContext for the rest of a typical
>> deployer config (simple should be easy)
>>
>> * Continue the use of bean xml file override for more complex behavior
>> configuration (complex is possible)
>>
>> * Create new property placeholders and defaults for bean properties
>> that could benefit from the new approach (e.g. SSO Session timeouts,
>> SLO on/off). (minimize the number of files that need to be managed in
>> the overlay)
>>
>> * Move the propertyFileConfigurer configuration block into
>> deployConfigContext.xml (minimize config files)
>>
>> * Provide robust sample config in cas.properties for all of the
>> properties that a deployment might want to override in the simple
>> case. (consistent approach)
>>
>> Bill
>>
>>
>> On Tue, Jan 3, 2012 at 5:49 PM, William G. Thompson, Jr.
>> <wgt...@gmail.com> wrote:
>> > On Tue, Jan 3, 2012 at 4:53 PM, Scott Battaglia
>> > <scott.battag...@gmail.com> wrote:
>> >>> Spring 3.x approach:
>> >> Q: "Hey, there's this placeholder ${ssoSession.maxTimeToLive:3600} in
>> >> ticketExpirationPolicies.xml, where's the value for that set?"
>> >> A: "Oh look, the default is right there in-line, bet I could override
>> >> that in cas.properties!"
>> >>
>> >> Your Q/A thing highlighted two things for me:
>> >> 1. We need better standards for property naming if we really name
>> things
>> >> without the units (i.e. maxTimeToLiveInSeconds) :-)
>> >
>> > +1 for well named properties.
>> >
>> >
>> >> 2. Having people read through all the Spring XML files rather than
>> >> default.properties to locate the values that can be
>> changed/paramaterized
>> >> seems like more work for the deployer. I would think properly named
>> >> placeholder names all located in one file would provide enough context
>> as
>> >> well as give us a single point of reference when questions come up.
>> Do we
>> >> want people to care which XML file the properties are in if they don't
>> plan
>> >> on overlaying those files?
>> >
>> > I think the reference issue can be addressed by having robust sample
>> > config and explanation right there in cas.properties for all the
>> > properties one might want to override (self documenting in a way). As
>> > a developer/deployer I'm attracted to the default values set right
>> > there in context.
>> >
>> > On balance both approaches seem to achieve the same results albeit
>> > with slightly different qualities. The Spring 3.x approach does it
>> > with one less file. Simpler == better?
>> >
>> > Either way, I think this is an important (thought minor) improvement,
>> > and I'm willing to take on this work for 3.5 regardless of which way
>> > consensus takes us.
>> >
>> >>
>> >> I think you and I are in agreement though that its time to get it right
>> >> (even if we don't completely agree on what "right" is ;-))
>> >
>> > Indeed.
>> >
>> > Bill
>> >
>> >
>> >>
>> >> Cheers,
>> >> Scott
>> >>
>> >>
>> >> On Tue, Jan 3, 2012 at 4:44 PM, William G. Thompson, Jr. <
>> wgt...@gmail.com>
>> >> wrote:
>> >>>
>> >>> What started this discussion was a desired to improve maven overlay
>> >>> based CAS deployments for the 3.5 release. The release strategy
>> >>> states that in a minor release (i.e. 3.4.x -> CAS 3.5) "CAS maven
>> >>> overlays may require minor to moderate changes". So, I tend to want
>> >>> to get this right for 3.5 even if it requires some minor to moderate
>> >>> changes to existing deployments.
>> >>>
>> >>> Generally, the first thing I do is externalize cas.properties with the
>> >>> goal of having a single war file that can be deployed to multiple
>> >>> tiers/nodes without further configuration (i.e. deploy/unpack the war,
>> >>> find the right config files, edit/copy the old ones, etc). Having
>> >>> some configuration parameters in cas.properties file is great, having
>> >>> it in embedded in the war file makes it less convenient for
>> >>> single-war-multiple-deployments.
>> >>>
>> >>> Overtime and many enterprise CAS deployments, I've also noticed that
>> >>> logging configuration could also benefit from externalization, which
>> >>> gave rise to CAS-1082, subsequent discussions including this one, and
>> >>> the discovery that default values for bean configuration could be
>> >>> in-lined like so:
>> >>>
>> >>> <property name="arguments">
>> >>> <list>
>> >>> <value>${log4j.config.location:classpath:log4j.xml}</value>
>> >>> <value>${log4j.refresh.interval:60000}</value>
>> >>> </list>
>> >>> </property>
>> >>>
>> >>> We also have the suggestion by Daniel Frett that a default.properties
>> >>> be added and loaded before cas.properties to allow default settings to
>> >>> be set for stock CAS and not cause issues for deployers when they
>> >>> override cas.properties in their own maven overlay.
>> >>>
>> >>> I agree with Scott's comments regarding coming up with a general
>> >>> strategy going forward (3.5+). So, here's a stab at an approach for
>> >>> this minor but helpful improvement.
>> >>>
>> >>> CAS Configuration should follow the following principles:
>> >>>
>> >>> * Simple should be easy, complex should be possible
>> >>> * Key config should be easily externalized so that a single war file
>> >>> can easily be deployed to multiple tiers or nodes
>> >>> * Consistent approach
>> >>> * Generally seek to limit the number of files that need to be managed
>> >>> in the overlay to make upgrading easier.
>> >>>
>> >>> A possible approach:
>> >>>
>> >>> * Push all defaults to the bean files where they are defined using the
>> >>> Spring 3.x approach above and continue to use cas.properties as the
>> >>> deployment override file for simple parameter configuration. Continue
>> >>> to use deployerConfigContext for the rest of a typical deployer config
>> >>> (simple should be easy)
>> >>>
>> >>> * Continue the use of bean xml file override for more complex behavior
>> >>> configuration (complex is possible)
>> >>>
>> >>> * Create new property placeholders and defaults for bean properties
>> >>> that could benefit from the new approach (e.g. SSO Session timeouts,
>> >>> SLO on/off). (minimize the number of files that need to be managed in
>> >>> the overlay)
>> >>>
>> >>> * Move the propertyFileConfigurer configuration block into
>> >>> deployConfigContext.xml (minimize the number of files that need to be
>> >>> managed in the overlay)
>> >>>
>> >>> Spring 3.x approach:
>> >>> Q: "Hey, there's this placeholder ${ssoSession.maxTimeToLive:3600} in
>> >>> ticketExpirationPolicies.xml, where's the value for that set?"
>> >>> A: "Oh look, the default is right there in-line, bet I could override
>> >>> that in cas.properties!"
>> >>>
>> >>> Bill
>> >>>
>> >>>
>> >>> On Tue, Jan 3, 2012 at 11:05 AM, Scott Battaglia
>> >>> <scott.battag...@gmail.com> 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:
>> >>> >> > 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:
>> >>> >> 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:
>> >>> > wgt...@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:
>> >>> 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:
>> wgt...@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:
>> 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:
> dmitriy.kopyle...@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
