On 19/02/13 07:05, David Strauss wrote:
On Mon, Feb 18, 2013 at 11:28 AM, Steven Hiscocks
<[email protected]> wrote:
Sounds good. I suppose `log_level` should become `add_loglevel_matches` for
consistency. (or actually `add_priority_matches`?)

"Log level" is the Pythonic language; please continue using it. The
journal itself may even move to it because "priority" is
counter-intuitive.

I was missing the default value of None in `this_machine`. It's supposed to
be used: "myjournal.this_machine()" (or "myjournal.add_machine_match()" with
above changes). Alternatively, the default None could be removed , and
constants set `CURRENT_MACHINE=id128.get_machine()`, etc., which then should
be used when calling the method?

That sounds even better. Presumably, id128.get_machine() is not too
heavy to call once each time the logging code gets imported?

I can imagine the overhead is negligible. Looking at the id128 code, it appears to just read "/etc/machine-id" and "/proc/sys/kernel/random/boot_id". (You could even get away with doing this in python ;) )
The ultimate would be CURRENT_MACHINE=<something-constant> and a
function that lazy-loads id128.get_machine() on the first request. It
may be a premature optimization for performance, but it's nice to be
able to import a Python module without worrying about it throwing an
exception initializing its constants.

I put in place holder for now for the constants using the id128 functions. Might be something to change if people have issues...
I'll add this to the documentation, but I think `get_next` should stay, as
it compliments `get_previous` and use of the `skip` value.

Makes sense.

Sure. I'll put in `UnicodeDecodeError`. I'm undecided on whether the call of
`self.converters[key](value)` exception handling should be limited to
`KeyError`? This would mean that the whole log entry would fail to return if
one of the converters failed.

Even a set of explicit exception classes is okay. Something that's a
complete surprise will still bubble up.

I've left the current code with catch all fall back to Unicode. It doesn't seem right that a whole log entry fails to return, just because one field is skewif.
--

I've pushed a few more commits.

--
Steven Hiscocks
_______________________________________________
systemd-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to