On 04/28/2016 09:30 AM, Lukas Slebodnik wrote: > On (27/04/16 15:18), Stephen Gallagher wrote: >> On 04/27/2016 05:57 AM, Pavel Březina wrote: >>> On 04/26/2016 05:08 PM, Stephen Gallagher wrote: >>>> Our users constantly make the mistake of typing `debug = 9` in the >>>> sssd.conf >>>> instead of `debug_level = 9` as would be correct. This happens >>>> frequently-enough >>>> that we should just alias it rather than continue to have people make >>>> mistakes. >>>> >>>> Resolves: >>>> https://fedorahosted.org/sssd/ticket/2999 >>> >>> I don't really oppose but I'd rather print a warning instead of aliasing it, >>> otherwise we can end up aliasing everything. It may be done as part of >>> configuration check patches that should detect typos.' >> >> >> Yeah, I don't want this to become a common thing (we shouldn't really be >> aliasing anything), but this is such a *common* mistake that it's bordering >> on >> ridiculous not to just make an exception here. >> >> When you get right down to it, most projects use the more abbreviated term >> "debug" anyway, so we're kind of the outlier. >> >> (You will notice I intentionally didn't add it to the manual; this is meant >> to >> be a hidden convenience feature, not the primary method. Also, `debug_level` >> will always overrule `debug` if both are present.) >> > I don't prefer undocumeted options. > If we really want to have an alias then it should be documented. > Otherwise users might wonder why it magicaly works. > > The option/alias will need to be added to the list of valid options anyway > with the config validation (coming soon) >
Ok, I added documentation and tests for it. See attached patch.
From 4a753d3a754ffa5a3e48eb6ba2fd8ebc73fd20be Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <[email protected]> Date: Tue, 26 Apr 2016 11:04:36 -0400 Subject: [PATCH] DEBUG: Add `debug` alias for debug_level Our users constantly make the mistake of typing `debug = 9` in the sssd.conf instead of `debug_level = 9` as would be correct. This happens frequently-enough that we should just alias it rather than continue to have people make mistakes. Resolves: https://fedorahosted.org/sssd/ticket/2999 --- src/confdb/confdb.h | 1 + src/config/SSSDConfig/__init__.py.in | 1 + src/config/SSSDConfigTest.py | 3 +++ src/config/etc/sssd.api.conf | 2 ++ src/man/sssd.conf.5.xml | 13 +++++++++++++ src/util/server.c | 15 ++++++++++++++- 6 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index a9b1c4362b5c0c6b158830b1bf2ef68db09d8d06..153e68a0761463723945667004b4505acbc5a0b9 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -51,10 +51,11 @@ /* Services */ #define CONFDB_SERVICE_PATH_TMPL "config/%s" #define CONFDB_SERVICE_COMMAND "command" #define CONFDB_SERVICE_DEBUG_LEVEL "debug_level" +#define CONFDB_SERVICE_DEBUG_LEVEL_ALIAS "debug" #define CONFDB_SERVICE_DEBUG_TIMESTAMPS "debug_timestamps" #define CONFDB_SERVICE_DEBUG_MICROSECONDS "debug_microseconds" #define CONFDB_SERVICE_DEBUG_TO_FILES "debug_to_files" #define CONFDB_SERVICE_TIMEOUT "timeout" #define CONFDB_SERVICE_FORCE_TIMEOUT "force_timeout" diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 1a0893cbc180394d994b9d97fc0fa863da656549..1c490cf714e06e75b1ce8d5dee1aec27dfc2d12b 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -38,10 +38,11 @@ else: _ = translation.ugettext # TODO: This needs to be made external option_strings = { # [service] + 'debug' : _('Set the verbosity of the debug logging'), 'debug_level' : _('Set the verbosity of the debug logging'), 'debug_timestamps' : _('Include timestamps in debug logs'), 'debug_microseconds' : _('Include microseconds in timestamps in debug logs'), 'debug_to_files' : _('Write debug messages to logfiles'), 'timeout' : _('Ping timeout before restarting service'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index e518c756599be42dddf3397a47451ac6afa05d29..6ec30234e24b7b48ccab6a98e1f9396990509190 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -297,10 +297,11 @@ class SSSDConfigTestSSSDService(unittest.TestCase): 're_expression', 'full_name_format', 'krb5_rcache_dir', 'user', 'default_domain_suffix', + 'debug', 'debug_level', 'debug_timestamps', 'debug_microseconds', 'debug_to_files', 'command', @@ -495,10 +496,11 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase): # First test default options options = domain.list_options() control_list = [ 'description', + 'debug', 'debug_level', 'debug_timestamps', 'min_id', 'max_id', 'timeout', @@ -861,10 +863,11 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase): # First test default options options = domain.list_options() control_list = [ 'description', + 'debug', 'debug_level', 'debug_timestamps', 'min_id', 'max_id', 'timeout', diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index a15f2bd05c3046f8d76b13b3d8f28f9001d8fded..e6756e6431c231f8f1dbe53227fc8ae3ea19b539 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -1,10 +1,11 @@ # Format: # option = type, subtype, mandatory[, default] [service] # Options available to all services +debug = int, None, false debug_level = int, None, false debug_timestamps = bool, None, false debug_microseconds = bool, None, false debug_to_files = bool, None, false command = str, None, false @@ -103,10 +104,11 @@ hostid_provider = str, None, false subdomains_provider = str, None, false [domain] # Options available to all domains description = str, None, false +debug = int, None, false debug_level = int, None, false debug_timestamps = bool, None, false command = str, None, false min_id = int, None, false max_id = int, None, false diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 09db9cd32673c911991b335e986692e3d8d856d0..6935685da64f202131f2055a25dc9402f199b750 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -68,10 +68,23 @@ <varlistentry> <term>debug_level (integer)</term> <xi:include xmlns:xi="http://www.w3.org/2001/XInclude"; href="include/debug_levels.xml" /> </varlistentry> <varlistentry> + <term>debug (integer)</term> + <listitem> + <para> + SSSD 1.14 and later also includes the + <replaceable>debug</replaceable> alias for + <replaceable>debug_level</replaceable> as a + convenience feature. If both are specified, the + value of <replaceable>debug_level</replaceable> + will be used. + </para> + </listitem> + </varlistentry> + <varlistentry> <term>debug_timestamps (bool)</term> <listitem> <para> Add a timestamp to the debug messages. If journald is enabled for SSSD debug logging this diff --git a/src/util/server.c b/src/util/server.c index 67a25955783c30dc03f3d6d9193c8547c625f134..074dc34848ae2b8dd98bf6b218cc6c9c8441104d 100644 --- a/src/util/server.c +++ b/src/util/server.c @@ -565,18 +565,31 @@ int server_setup(const char *name, int flags, if (debug_level == SSSDBG_UNRESOLVED) { /* set debug level if any in conf_entry */ ret = confdb_get_int(ctx->confdb_ctx, conf_entry, CONFDB_SERVICE_DEBUG_LEVEL, - SSSDBG_DEFAULT, + SSSDBG_UNRESOLVED, &debug_level); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Error reading from confdb (%d) " "[%s]\n", ret, strerror(ret)); return ret; } + if (debug_level == SSSDBG_UNRESOLVED) { + /* Check for the `debug` alias */ + ret = confdb_get_int(ctx->confdb_ctx, conf_entry, + CONFDB_SERVICE_DEBUG_LEVEL_ALIAS, + SSSDBG_DEFAULT, + &debug_level); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Error reading from confdb (%d) " + "[%s]\n", ret, strerror(ret)); + return ret; + } + } + debug_level = debug_convert_old_level(debug_level); } /* same for debug timestamps */ if (debug_timestamps == SSSDBG_TIMESTAMP_UNRESOLVED) { -- 2.7.4
signature.asc
Description: OpenPGP digital signature
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
