Edgewall Software

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13121 closed defect (fixed)

"Error: Invalid log level" when switching Logging Type to None in Admin panel — at Version 12

Reported by: oliver.joos@… Owned by: Jun Omae
Priority: normal Milestone: 1.2.4
Component: admin/web Version: 1.2.3
Severity: major Keywords:
Cc: oliver.joos@… Branch: log:jomae.git@t13121.1+1.2-stable log:jomae.git@t13121.1+trunk
Release Notes:

Fix TracError raising and keep log level and log file options when the log type is none.

API Changes:
Internal Changes:

Description

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

Change History (12)

comment:1 by Ryan J Ollos, 4 years ago

Milestone: 1.2.4
Owner: set to Ryan J Ollos
Status: newassigned

Confirmed. Behavior occurs after r15783 (#8790).

comment:2 by Jun Omae, 4 years ago

Branch: log:jomae.git@t13121+1.2-stable

comment:3 by Ryan J Ollos, 4 years ago

Change looks good to me. Thanks for fixing.

comment:4 by Jun Omae, 4 years ago

Owner: changed from Ryan J Ollos to 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.

in reply to:  4 comment:5 by Ryan J Ollos, 4 years ago

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.

comment:6 by Jun Omae, 4 years ago

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

comment:7 by anonymous, 4 years ago

@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.

in reply to:  7 comment:8 by Jun Omae, 4 years ago

Maybe for Trac 1.3.x?

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

comment:9 by Ryan J Ollos, 4 years ago

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

    diff --git a/trac/admin/web_ui.py b/trac/admin/web_ui.py
    index 3651e5a3e..39ae34716 100644
    a b from genshi.builder import tag  
    2525from trac.admin.api import IAdminPanelProvider
    2626from trac.core import *
    2727from trac.loader import get_plugin_info
    28 from trac.log import LOG_LEVELS, LOG_LEVEL_ALIASES
     28from trac.log import LOG_LEVELS, LOG_LEVEL_ALIASES, LOG_LEVEL_ALIASES_MAP
    2929from trac.perm import PermissionSystem, IPermissionRequestor
    3030from trac.util.datefmt import all_timezones, pytz
    3131from trac.util.text import exception_to_unicode, \
    class LoggingAdminPanel(Component):  
    333333                _save_config(self.config, req, self.log),
    334334            req.redirect(req.href.admin(cat, page))
    335335
     336        log_level = LOG_LEVEL_ALIASES_MAP.get(log_level, log_level)
     337
    336338        data = {
    337339            'type': log_type, 'types': log_types,
    338340            'level': log_level, 'levels': LOG_LEVELS,
  • trac/log.py

    diff --git a/trac/log.py b/trac/log.py
    index 3f00f98f7..a98473885 100644
    a b LOG_TYPE_ALIASES = ('winlog', 'nteventlog', 'unix')  
    2424LOG_LEVELS = ('CRITICAL', 'ERROR', 'WARNING', 'INFO', 'DEBUG')
    2525LOG_LEVEL_ALIASES = ('WARN', 'ALL')
    2626
     27LOG_LEVEL_ALIASES_MAP = {'WARN': 'WARNING', 'ALL': 'DEBUG'}
    2728
    2829def logger_handler_factory(logtype='syslog', logfile=None, level='WARNING',
    2930                           logid='Trac', format=None):

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

comment:10 by Jun Omae, 4 years ago

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.

comment:11 by Ryan J Ollos, 4 years ago

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

comment:12 by Jun Omae, 4 years ago

Branch: log:jomae.git@t13121+1.2-stablelog:jomae.git@t13121.1+1.2-stable log:jomae.git@t13121.1+trunk
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

Note: See TracTickets for help on using tickets.