Add --log-level and LogControl1 support #482
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| void | ||
| polkit_backend_authority_set_log_level (const gchar *level) | ||
| { | ||
| if (g_strcmp0 (level, "debug") == 0) |
There was a problem hiding this comment.
This doesn't allow using numerical values for shortopt. Using sth like "polkitd -l crit" doesn't look like a standard solution.
There was a problem hiding this comment.
I have never seen any project expose this with numeric values? In systemd we do use the string names though, see SYSTEMD_LOG_LEVEL and LogControl1
https://www.freedesktop.org/software/systemd/man/latest/systemd.html#%24SYSTEMD_LOG_LEVEL
https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.LogControl1.html
I don't think it's necessary to expose this option as a numeric value for users, it's something that programmers might understand, but it would be very opaque for a normal user
There was a problem hiding this comment.
Sorry, but using numerical values would be confusing (for administrators, mind you), but using sometimes shortened words for shortopts wouldn't?
This is not an intention to massage my ego, but it simply doesn't make sense or consistency.
I've seen many standard programs that use shortopt with a number and longopt with a string.
If polkitd wasn't such a small program expected to be used frequently from shell directly, I'd be for creating one GOption "-l" with nums and another one for "--log-levels" for textual parameters, which would be consistently uncut.
There was a problem hiding this comment.
I meant for log levels. I don't remember anything common enough that takes parameters for the log level as an opaque number. The enum is an implementation detail of syslog after all, the way it's exposed to users is via these strings
| polkit_authority_log_level = (guint) LOG_LEVEL_NOTICE; | ||
| else if (g_strcmp0 (level, "warning") == 0) | ||
| polkit_authority_log_level = (guint) LOG_LEVEL_WARNING; | ||
| else if (g_strcmp0 (level, "err") == 0) |
There was a problem hiding this comment.
Some options are abbreviated, some are not. Inconsistency brings confusion.
There was a problem hiding this comment.
Yeah these mixed abbreviations are very strange and I don't understand why they are named like that, however, that's just how syslog levels are called and they are pretty much the standard, and how they are called in the LogControl1 protocol, in all log parsers and viewers like journald and journalctl (ie: you do journalctl -p err rather than -p error) - I think consistency between ways to configure and use this and across the ecosystem is important here, more than aesthetic on an individual project level, although if I could go back in time I'd fix them to be consistently named:
https://man7.org/linux/man-pages/man3/syslog.3.html
https://www.freedesktop.org/software/systemd/man/latest/systemd.html#%24SYSTEMD_LOG_LEVEL
https://www.freedesktop.org/software/systemd/man/latest/org.freedesktop.LogControl1.html
There was a problem hiding this comment.
But the syslog manpage, from which LogControl got apparently inspired, communicates on the programmer level, not on the level of UI. Deriving textual parameters used directly by user from program's enum seems like a UI mistake made long ago. Do we want to continue in this evil? Who do we make this for? For admins who would be confused by numbers but not by half-words derived from enums?
I say we should use whole words for UI, program internals regardless, while we're at it.
There was a problem hiding this comment.
It needs to be the same yes, because again that's what user interfaces like systemctl and journalctl expose, so it's worse for usability to diverge: https://www.freedesktop.org/software/systemd/man/latest/systemctl.html#service-log-level%20SERVICE%20%5BLEVEL%5D
What we can do, is accept both versions as input, as aliases. That's easy to do, I'll push shortly.
Alternative to #477