eliot icon indicating copy to clipboard operation
eliot copied to clipboard

log_call could be more useful if it interoperated with the type system

Open exarkun opened this issue 6 years ago • 6 comments

log_call is appealing for the effort/lines it saves. But using it requires giving up on the Field system.

If I have an ActionType FOO defined, it would be nice to be able to say:

@log_call(action_type=FOO)
def bar(...):
    ...

And have this be equivalent to:

def bar(...):
    with FOO(...):
        ...

exarkun avatar Feb 21 '19 19:02 exarkun

Or possibly an API like:

@log_call(action_type=u"foo", x=SOME_FIELD, ...)
def foo(x, ...):
    ...

Since it's fields, not actions, that actually define the validators/serializers.

exarkun avatar Feb 22 '19 17:02 exarkun

Two use cases I can see here:

  1. You want type for documentation/type checking. In this case, Python's type hints and related toolchain seems better, otherwise you're basically duplicating information.
  2. Testing. For this use case, note that I'm pretty sure assertHasAction can now take action_type strings, not just ActionType objects. Would that address your requirements?

itamarst avatar Feb 27 '19 19:02 itamarst

You want type for documentation/type checking. In this case, Python's type hints and related toolchain seems better, otherwise you're basically duplicating information.

On reflection, I think my angle is really more about the serializers. I may be able to use some other tool to ensure arguments have the right type but for anything other than JSON-compatible types, Eliot is still unable to serialize them.

2. Testing. For this use case, note that I'm pretty sure assertHasAction can now take action_type strings, not just ActionType objects.

Hm. I haven't been writing a lot of tests for the logging I've been adding, yet. So I'm not sure about this one. Certainly it would be bad if you couldn't use assertHasAction to make assertions about an action emitted by log_call instead of one of the other ways. So it sounds like a good thing that it can work on action_type strings now. But beyond that, I dunno.

exarkun avatar Feb 27 '19 19:02 exarkun

Ah, serializers, I forgot about those. It's possible to have a custom JSON encoder class, so you can add custom serialization logic that way (and at work I added support for a __eliot__ on objects), but serializers are much less global, so I can see the benefit.

itamarst avatar Feb 27 '19 19:02 itamarst

I like the @functools.singledispatch trick that Hynek discusses. The key is passing

json.dumps(obj, default=to_serializable)

Here's an implementation:

import functools
import json
import io
import sys


from typing import Any
from typing import Optional
from typing import Union
from typing import Callable


import attr
import eliot
import pendulum


@functools.singledispatch
def to_serializable(obj: Any) -> Union[str, list, dict, int, float]:
    try:
        return attr.asdict(obj)
    except attr.exceptions.NotAnAttrsClassError:
        return str(obj)


@to_serializable.register(pendulum.DateTime)
def _(obj):
    return obj.to_iso8601_string()


def json_to_file(file: Optional[io.TextIOWrapper] = None) -> Callable:

    if file is None:
        file = sys.stdout

    def _json_to_file(x):
        print(json.dumps(x, default=to_serializable), file=file)

    return _json_to_file


eliot.add_destination(json_to_file)

Considering switching to multipledispatch as it can use the type annotations on the function arguments instead of requiring register(T), and can have multiple registries.

jtrakk avatar Mar 23 '19 20:03 jtrakk

In some ways that's nicer, yeah. The real question to me is how global vs. specific serialization can work. And maybe the answer is "specific can be done a bit more manually" except that doesn't work for log_call...

itamarst avatar Mar 24 '19 12:03 itamarst