New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`JSONRenderer(default=custom_json_fallback_handler)` results in `TypeError: dumps() got multiple values for keyword argument 'default'` #77

Closed
groner opened this Issue Aug 18, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@groner
Contributor

groner commented Aug 18, 2016

I use the JSONEncoder() default argument to handle rendering datetime using ISO format. With 9dd4d65 JSONRenderer() provides a default keyword argument, and providing my own results in the TypeError I described in the title.

This could be fixed by having JSONRenderer mix the _json_fallback_handler default into self._dumps_kw.

Is there another way I should be doing this? I already use the TimeStamper processor, but I have other datetime objects that I would prefer not to render as repr. Maybe structlog could provide a functools.singledispatch callable as an alternative to __structlog__ methods?

@hynek

This comment has been minimized.

Owner

hynek commented Aug 19, 2016

Hmm I think I had to switch from cls to default, because it wasn’t as widely supported?

At this point JSONEncoder is 2 LoC so I don't want to do too much magic around it, since it’s easier to write an own one.


That said I’ve been thinking about a better serialization for a while. I haven’t used single dispatch but would that mean that I provide something like “to_dict” and users can do a “@to_dict.register” on top of it? That actually sounds pretty great if it’d work!

@groner

This comment has been minimized.

Contributor

groner commented Aug 19, 2016

That makes sense, subclassing json.JSONEncoder probably doesn't affect other json libraries.


That's basically it, they would do something like @to_dict.register(datetime) to handle calls where the first argument is a datetime instance. It's not in 2.7, but the author has a backport on pypi.

For the case at hand, to_dict is too specific, since I want to convert datetimes to an ISO string. Maybe something like to_log?

@hynek

This comment has been minimized.

Owner

hynek commented Aug 20, 2016

to_json_dumpable or similar would be better I guess because it doesn’t necessarily make sense with all loggers.

But man, I feel like we’re on to something here because the mechanism of default fallbacks in JSON is something that filled me with rage and frustration since I’ve started using the json library. Using single dispatch never occurred to me as an elegant solution.

I’ve just looked it up and found a

def to_serializable(val):
    """
    Transform *val* into something that can be serialized to JSON. Falls back
    to str.
    """
    if isinstance(val, datetime):
        return val.isoformat() + "Z"
    elif isinstance(val, enum.Enum):
        return val.value
    elif attr.has(val.__class__):
        return attr.asdict(val)
    elif isinstance(val, Exception):
        return {
            "error": val.__class__.__name__,
            "args": val.args,
        }
    return str(val)

in one of my projects (it is used both by structlog but also for web views). I’ve basically re-implemented single dispatch in a central place.

I’ve just rewritten it as:

@singledispatch
def to_serializable(val):
    """
    Transform *val* into something that can be serialized to JSON.
    """
    if attr.has(val.__class__):
        return attr.asdict(val)

    return str(val)


@to_serializable.register(datetime)
def s_datetime(val):
    return val.isoformat() + "Z"


@to_serializable.register(enum.Enum)
def s_enum(val):
    return val.value


@to_serializable.register(Exception)
def s_exc(val):
    return {
        "error": val.__class__.__name__,
        "args": val.args,
    }

which works just as good but is nicely decentralized. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment