"Error: Invalid log level" when switching Logging Type to None in Admin panel

[oliver.joos@…]

How?

If I go to Admin / Logging and choose Type "File" and "Apply changes" it works as expected and says "Your changes have been saved". But if I choose Type "None" again and "Apply changes" then and error page shows with "Unknown log level None" in a red box.

Where?

I see this bug in trac-1.2.3, installed with pip of Python 2.7.9 on an up-to-date Raspbian Stretch.

And I can reproduce this bug on a fresh ISO-DVD of Linux Mint 19.0 (Python 2.7.15) and a Trac built from source of rjollos.git branch "pull_de_from_transifex" (I guess one of the latest rc of upcoming 1.2.4)

Workaround

As root I edit trac.ini manually. But without access to trac.ini one could leave Type as-is and switch Level to CRITICAL to mute Logging.

Solution

I think this bug should not be fixed in the page code, but here:

https://trac.edgewall.org/browser/tags/trac-1.2.3/trac/admin/web_ui.py#L288

I would change this line into:

new_level = req.args.get('log_level') or log_level

[Ryan J Ollos]

Confirmed. Behavior occurs after r15783 (#8790).

[Jun Omae]

Proposed changes in [6f64267ef/jomae.git] (jomae.git@t13121+1.2-stable).

[Ryan J Ollos]

Change looks good to me. Thanks for fixing.

[Jun Omae]

Thanks for the reviewing. I push the changes later.

However, I consider log_level and log_file options should be kept when none is selected in log_type option. Currently, log_level options is removed and log_file option is reset to trac.log.

[Ryan J Ollos]

Replying to Jun Omae:

However, I consider log_level and log_file options should be kept when none is selected in log_type option. Currently, log_level options is removed and log_file option is reset to trac.log.

Makes sense to fix, I think. I recall looking at the code and questioning if the behavior makes sense, but chose to leave as-is. At least, that is my recollection, and it looks like the behavior existed back in 0.12: tags/trac-0.12/trac/admin/web_ui.py@:270,293#L255​.

[Jun Omae]

Thanks. Proposed changes in [7c1709559/jomae.git] (jomae.git@t13121.1+1.2-stable) to keep the options when the log type is none.

[anonymous]

@Jun Omae: I agree! Keeping log_level and log_file would be nicer. Maybe for Trac 1.3.x?

Meanwhile one could use the workaround I wrote in the description above.

[Jun Omae]

Maybe for Trac 1.3.x?

Yeah. The changes would be applied to trunk (jomae.git@t13121.1+trunk).

[Ryan J Ollos]

Tested and changes look good.

I noticed that if log_level is WARN or ALL, the proper choice won't be selected. The following might be simple enough to fix:

trac/admin/web_ui.py

@@ -25 +25 @@
 from trac.admin.api import IAdminPanelProvider
 from trac.core import *
 from trac.loader import get_plugin_info
-from trac.log import LOG_LEVELS, LOG_LEVEL_ALIASES
+from trac.log import LOG_LEVELS, LOG_LEVEL_ALIASES, LOG_LEVEL_ALIASES_MAP
 from trac.perm import PermissionSystem, IPermissionRequestor
 from trac.util.datefmt import all_timezones, pytz
 from trac.util.text import exception_to_unicode, \
@@ -333 +333 @@
                 _save_config(self.config, req, self.log),
             req.redirect(req.href.admin(cat, page))
 
+        log_level = LOG_LEVEL_ALIASES_MAP.get(log_level, log_level)
+
         data = {
             'type': log_type, 'types': log_types,
             'level': log_level, 'levels': LOG_LEVELS,

trac/log.py

@@ -24 +24 @@
 LOG_LEVELS = ('CRITICAL', 'ERROR', 'WARNING', 'INFO', 'DEBUG')
 LOG_LEVEL_ALIASES = ('WARN', 'ALL')
 
+LOG_LEVEL_ALIASES_MAP = {'WARN': 'WARNING', 'ALL': 'DEBUG'}
 
 def logger_handler_factory(logtype='syslog', logfile=None, level='WARNING',
                            logid='Trac', format=None):

If log_level = WARN, it will be changed to log_level = WARNING on save, and same for ALL → DEBUG, but I think that's fine.

[Jun Omae]

Thanks for the suggestion. Revised the branch with your changes and minor changes in jomae.git@t13121.1+1.2-stable and jomae.git@t13121.1+trunk.

[Ryan J Ollos]

Thanks for adding the test. Tested and changes look good to me.

[Jun Omae]

Committed in [16884] and merged in [16885].