log_level and log_type should be ChoiceOptions

[Ryan J Ollos]

If we change log_level and log_type to ChoiceOptions, a TracError will be raised when an invalid value is used.

[logging]
log_level = DEBUG
log_type = stderror

Error

TracError: [logging] log_type: expected one of ("file", "none", "stderr", "syslog", "winlog"), got u'stderror'

This could be useful, but will be challenged by the presence of aliases:

• ALL​ is an alias for DEBUG, and winlog and syslog have aliases​.
• WARN​ is documented as an alias for WARNING, and it appears that the default is WARNING​ for any unrecognized value.

Possible resolutions are:

• Eliminate some aliases and replace them in trac.ini in a upgrade step.
• Enhance ChoiceOption to support aliases.

I probably won't take any action on this for a while, but prepared some proposed changes in log:rjollos.git:t11573 before noticing these issues, and wanted to capture them in a ticket.

[Jun Omae]

I think using ChoiceOption is a good idea. I like to enhance ChoiceOption rather than add a upgrade step for the minor issue.

Also, in [2e6e9c1c/rjollos.git], documents of the log_level and log_type options wouldn't be extracted for tracini.pot.

BTW, I think a ConfigurationError will be raised for such a misconfiguration, not a TracError.

[Ryan J Ollos]

Replying to jomae:

BTW, I think a ConfigurationError will be raised for such a misconfiguration, not a TracError.

ConfigurationError is raised in the call to trac.env.open_environment and the exception is trapped in trac.web.main.dispatch_request. The exception message is passed as env_error to trac.web.main._dispatch_request and an HTTPInternalError is raised: tags/trac-1.0.1/trac/web/main.py@:494#L485​. That exception is trapped (tags/trac-1.0.1/trac/web/main.py@:502#L485​) and sent to the user in trac.web.main._send_user_error. The result is that the user sees:

Error

TracError: [logging] log_type: expected one of ("file", "none", "stderr", "syslog", "winlog"), got u'stderror'

So yes, a ConfigurationError is raised. I should have been more precise and said "the user sees a TracError", rather than "a TracError will be raised".

[Ryan J Ollos]

[trac] default_date_format and [trac] default_dateinfo__format can also be ChoiceOptions: log:rjollos.git:t11573.1.

This results in a significant change in behavior in that a ConfigurationError is raised when an invalid option is specified in trac.ini (similar to the change in #10285).

It was suggested in comment:2:ticket:10285 to add a new class rather than having OrderedExtensionOption raise an exception. Reconsidering this behavior, it might make sense to have a strict parameter for the ChoiceOption, ExtensionOption and OrderedExtensionOption. The strict option would ultimately determine whether the error is only logged, or also presented to the UI. The mechanism could be to have two exception classes that get handled differently.

[Ryan J Ollos]

The changes in log:rjollos.git:t11573.1 could be pushed to the trunk rather than 1.0-stable so that we have time to get feedback on whether it's too extreme of a reaction to show a ConfigurationError on the web interface in response to an invalid configuration value.

[Ryan J Ollos]

Changes from comment:4 committed to trunk in [13298].