From d2231b8c4a14c305c8b9a3f2f635b9daf54af46f Mon Sep 17 00:00:00 2001 From: Iustin Pop <iustin@google.com> Date: Tue, 3 Nov 2009 09:20:35 +0100 Subject: [PATCH] Change behaviour of ConfigWriter._WriteConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch changes the behaviour of _WriteConfig in case of configuration errors: - before, it used to abort the saving (even though the in-memory configuration used by current jobs has already changed) - now, we log it (both to the log and to the user) but continue, since we can't revert to a good version of the config anyway This should make the internal behaviour of the code more consistent with the external world, even though the config might be βwrongβ; we leave the cleanup to the user. This should not be as bad as it sounds, since we haven't actually seen this case except for the ugly master candidates handling, and that was fixed recently by Guido's patch series. Signed-off-by: Iustin Pop <iustin@google.com> Reviewed-by: Michael Hanselmann <hansmi@google.com> --- lib/config.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/config.py b/lib/config.py index c64b5264d..661e2e8e3 100644 --- a/lib/config.py +++ b/lib/config.py @@ -1195,15 +1195,23 @@ class ConfigWriter: """ assert feedback_fn is None or callable(feedback_fn) - # first, cleanup the _temporary_ids set, if an ID is now in the + # First, cleanup the _temporary_ids set, if an ID is now in the # other objects it should be discarded to prevent unbounded growth # of that structure self._CleanupTemporaryIDs() + + # Warn on config errors, but don't abort the save - the + # configuration has already been modified, and we can't revert; + # the best we can do is to warn the user and save as is, leaving + # recovery to the user config_errors = self._UnlockedVerifyConfig() if config_errors: - raise errors.ConfigurationError("Configuration data is not" - " consistent: %s" % - (", ".join(config_errors))) + errmsg = ("Configuration data is not consistent: %s" % + (", ".join(config_errors))) + logging.critical(errmsg) + if feedback_fn: + feedback_fn(errmsg) + if destination is None: destination = self._cfg_file self._BumpSerialNo() -- GitLab