Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#13121 closed defect (fixed)

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

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:

Fixed TracError exception and preserve log level and log file options when the log type is changed to 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

Attachments (0)

Change History (14)

comment:1 by Ryan J Ollos, 5 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, 5 years ago

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

comment:3 by Ryan J Ollos, 5 years ago

Change looks good to me. Thanks for fixing.

comment:4 by Jun Omae, 5 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, 5 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, 5 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, 5 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, 5 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, 5 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, 5 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, 5 years ago

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

comment:12 by Jun Omae, 5 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].

comment:13 by Ryan J Ollos, 5 years ago

Release Notes: modified (diff)

comment:14 by Ryan J Ollos, 5 years ago

Release Notes: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.