[systemd-devel] Python journal reader

[David Strauss]

I keep writing lengthy emails about how we can use this as an
opportunity to reduce redundancy and improve consistency, but I should
probably ping you on #systemd IRC to hash it out. I can't think of
anything elegant that doesn't involve altering the existing journal.py
or _reader.c code.

What's your handle? Mine's davidstrauss.

As a starter, I want to enumerate the places where
__REALTIME_TIMESTAMP has special handling:
 (1) Existing code: A C-based member function in _reader.c.
 (2) Existing code: A native Python conversion function in journal.py.
 (3) Existing code: An entry in DEFAULT_CONVERTERS.
 (4) Added in your patch: A key setting in get_next().
 (5) Added in your patch: A Python-based member function in journal.py
that overrides (1).
 (6) Added in your patch: A condition in get() that invokes (1) when
the specific field is requested.

While I want to fix this bug, I also don't want six scattered special
cases for __REALTIME_TIMESTAMP and similar fields.

On Sat, Apr 13, 2013 at 4:18 PM, Steven Hiscocks
<steven-systemd at hiscocks.me.uk> wrote:
> On 13/04/13 23:47, Steven Hiscocks wrote:
>>
>> On 13/04/13 23:00, David Strauss wrote:
>>>
>>> If seems like we should put the conditional special handling for
>>> __REALTIME_TIMESTAMP and __MONOTONIC_TIMESTAMP in either _reader.c or
>>> right in get().
>>>
>>> Here's why:
>>>   * With the code above, calling Reader.get('__REALTIME_TIMESTAMP')
>>> results in the wrong type (if anything, I'd need to check), but
>>> Reader.get_realtime() gives the right thing.
>>>   * The type of Reader.get('__REALTIME_TIMESTAMP') is different from
>>> Reader.get_next()['__REALTIME_TIMESTAMP'], and it should be
>>> equivalent.
>>>
>> Good point. Currently, `get` for realtime, monotonic or cursor do not
>> return anything, but raise an exception (invalid field). Given the plan
>> to keep things in `_Reader` close to the C API, I'd suggest adding the
>> handling within the `Reader.get`. Suggestion merged below:
>>
>> ```
>> ```
>>>
>>> On Fri, Apr 12, 2013 at 10:13 AM, Steven Hiscocks
>>> <steven-systemd at hiscocks.me.uk> wrote:
>>>>
>>>> Hi,
>>>>
>>>> In the python journal Reader,  the splitting out of monotonic and
>>>> realtime
>>>> stamps, has affected `get_next` function as timestamp values are no
>>>> longer
>>>> present in the dictionary returned. Also the new `get_monotonic` and
>>>> `get_realtime` functions are not run through the converters. Equally,
>>>> the
>>>> 'get' method added is not run through the converters. I also noted the
>>>> additional `next` method doesn't work on python2, as it clashes with the
>>>> iter `next` method (python3 not affected as it changes iter method to
>>>> `__next__`)
>>>>
>>>> My suggestion with the python Reader `get_next` method is that the
>>>> realtime
>>>> and monotonic timestamps remain part of it, as these are key parts of
>>>> a log
>>>> entry, and two more lines in everyone's code to get them seems
>>>> cumbersome.
>>>> Equally also the cursor value. This also makes the output fields the
>>>> same as
>>>> the journalctl json format. (I agree it makes sense the _Reader object
>>>> element to remain separate so close to actual C API).
>>>>
>>>> I'm not sure what the best approach to the `next` method issue is…
>>>>
>>>> Proposed changes below. I've add `get_cursor` to go through
>>>> converters for
>>>> consistency, even if not required currently.
>>>>
>>>> ```
>>>> ```
>>>>
>>>> Thanks
>>>> --
>>>> Steven Hiscocks
>>>> _______________________________________________
>>>> systemd-devel mailing list
>>>> systemd-devel at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>>>
>>>
>>>
>>>
>>
>>
> Just noticed an unintended effect if _Reader.get_next returns empty
> dictionary. Fixed:
>
> ```
> diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py
> index 48f57ac..281d7c3 100644
>
> --- a/src/python-systemd/journal.py
> +++ b/src/python-systemd/journal.py
> @@ -189,6 +189,28 @@ class Reader(_Reader):
>          for arg in args:
>              super(Reader, self).add_match(arg)
>
> +    def get(self, key):
> +        if key == "__REALTIME_TIMESTAMP":
> +            return self.get_realtime()
> +        elif key == "__MONOTONIC_TIMESTAMP":
> +            return self.get_monotonic()
> +        elif key == "__CURSOR":
> +            return self.get_cursor()
> +        else:
> +            return self._convert_field(key, super(Reader, self).get(key))
> +
> +    def get_realtime(self):
> +        return self._convert_field(
> +            '__REALTIME_TIMESTAMP', super(Reader, self).get_realtime())
> +
> +    def get_monotonic(self):
> +        return self._convert_field(
> +            '__MONOTONIC_TIMESTAMP', super(Reader, self).get_monotonic())
> +
> +    def get_cursor(self):
> +        return self._convert_field(
> +            '__CURSOR', super(Reader, self).get_cursor())
> +
>      def get_next(self, skip=1):
>          """Return the next log entry as a dictionary of fields.
>
> @@ -197,8 +219,14 @@ class Reader(_Reader):
>
>          Entries will be processed with converters specified during
>          Reader creation.
>          """
> -        return self._convert_entry(
> +        entry = self._convert_entry(
>              super(Reader, self).get_next(skip))
> +        if entry:
>
> +            entry['__REALTIME_TIMESTAMP'] =  self.get_realtime()
> +            entry['__MONOTONIC_TIMESTAMP']  = self.get_monotonic()
> +            entry['__CURSOR']  = self.get_cursor()
> +
> +        return entry
>
>      def query_unique(self, field):
>          """Return unique values appearing in the journal for given `field`.
> ```
>
> --
> Steven Hiscocks



-- 
David Strauss
   | david at davidstrauss.net
   | +1 512 577 5827 [mobile]