> 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) :-)
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 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 ;-))

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: 
arch...@mail-archive.com
To unsubscribe, change settings or access archives, see 
http://www.ja-sig.org/wiki/display/JSG/cas-dev

Reply via email to