Re: [PATCH] Upgrade process for obsolete options

From: Alex Rousskov <rousskov_at_measurement-factory.com>
Date: Thu, 23 Sep 2010 09:57:00 -0600

On 09/23/2010 08:30 AM, Amos Jeffries wrote:
> One problem we currently have with upgrades is leaving the parser able
> to avoid its bungled/unknown option message for directives which have
> been fully removed or massively syntax altered.
>
> We are able to handle this for flags and option syntax easily but the
> parser has been particularly dense and strict on the directives (first
> word of each line).
>
> This patch updates the cf_* and cfgman code to allow a special directive
> type "obsolete" which pumps old config lines off to a parse_obsolete()
> function for handling without causing the directives to remain in the
> publicly visible config documentation.
>
> cf.data.pre has entries added for all the squid-2.6+ directives I could
> find that were removed up to 3.HEAD
>
> parse_obsolete(char*) is added with tests to catch and report the above
> entries as obsolete while ignoring. Several of the directives have
> additional explanation text matching the release notes to aid manual
> upgrades.

There is a clash of approaches here. I think it is best for Squid not to
start if the wrong option may significantly affect its functionality.
This is especially true during upgrades, when no sane admin would try to
upgrade without a backup readily available (even if they did not offline
checking). However, I know that many developers prefer when Squid
bypasses configuration errors and starts if at all possible instead.

I like the absolete feature as such. I would recommend allowing
cf.data.pre COMMENTs for obsolete options and recording upgrade
instructions there instead of hard-coding them in parse_obsolete(). A
generated obsolete option parser would then display those comments. That
would be a much cleaner solution, IMO.

It is fine to not even try upgrading automatically, IMO. If we want to
upgrade automatically, we should do it via a dedicated hand-written
upgrade_* function, referenced from the cf.data.pre blob of an obsolete
feature.

I cannot object to the proposed changes even though I think there is a
better way to support the same good idea.

Thank you,

Alex.
Received on Thu Sep 23 2010 - 15:57:02 MDT

This archive was generated by hypermail 2.2.0 : Thu Sep 23 2010 - 12:00:11 MDT