You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
My application makes use of SQLAlchemy and eliot. Today, I discovered that sqlalchemy's Table.name is a sqlalchemy.sql.elements.quoted_name, not a str, so I would find destination_failure objects with a TypeError: Dict key must be str error in the log tree when I tried something like this:
Since the dictionary shown in the destination_failure appeared to have all str keys, it took some docs-reading and a thorough debugger session to figure out that the exception is from orjson being exceedingly strict about the keys' types by default. It does not rely on the __str__ protocol like most generic code would, but seems to check that the key is exactlystr and not some subclass thereof.
A quick test serializing the same dict with orjson.dumps(results, option=orjson.OPT_NON_STR_KEYS) showed that the quoted_name type can be serialized without any further tweaking (assuming the stated performance penalty is acceptable), but there seems to be no way to set this option through eliot.
My proposal would be to add a json_option parameter to destination class initialization. I'm not familiar with Pyrsistent's PClass, so idk if this requires any translation, but eg. for FileDestination:
def__new__(cls, file, encoder=None, json_default=json_default, json_option=None): # new kwarg
...
returnPClass.__new__(
cls,
file=file,
_dumps=_dumps,
_linebreak=_linebreak,
_json_default=json_default,
_json_option=json_option, # passing kwarg to parent constructor
)
def__call__(self, message):
self.file.write(
self._dumps(message, default=self._json_default, option=self.json_option) +self._linebreak
) # using the new attributeself.file.flush()
Of course the destination type could be subclassed, but, imo, that's quite a lot of work for one extra option that could (should, imo) be baked in. The other generalized workaround is to wrap the destination type and stringify all the keys before passing the message dict to FileDestination, and I will probably do just that in the meantime since I already have a wrapper for something else, but I can't imagine that native Python could do it faster than orjson could--even accounting for performance penalty of OPT_NON_STR_KEYS--and I understood the switch to orjson to be motivated by performance... The last workaround I can think of is to just never use log_call on something that returns a dict whose keys have even a slight chance of not being exactly str, but if that were to be the "official" solution I would ask that it at least be documented.
The text was updated successfully, but these errors were encountered:
My application makes use of SQLAlchemy and eliot. Today, I discovered that sqlalchemy's
Table.name
is asqlalchemy.sql.elements.quoted_name
, not astr
, so I would finddestination_failure
objects with aTypeError: Dict key must be str
error in the log tree when I tried something like this:Since the dictionary shown in the
destination_failure
appeared to have allstr
keys, it took some docs-reading and a thorough debugger session to figure out that the exception is fromorjson
being exceedingly strict about the keys' types by default. It does not rely on the__str__
protocol like most generic code would, but seems to check that the key is exactlystr
and not some subclass thereof.A quick test serializing the same
dict
withorjson.dumps(results, option=orjson.OPT_NON_STR_KEYS)
showed that thequoted_name
type can be serialized without any further tweaking (assuming the stated performance penalty is acceptable), but there seems to be no way to set this option through eliot.My proposal would be to add a
json_option
parameter to destination class initialization. I'm not familiar with Pyrsistent'sPClass
, so idk if this requires any translation, but eg. forFileDestination
:Of course the destination type could be subclassed, but, imo, that's quite a lot of work for one extra option that could (should, imo) be baked in. The other generalized workaround is to wrap the destination type and stringify all the keys before passing the message
dict
toFileDestination
, and I will probably do just that in the meantime since I already have a wrapper for something else, but I can't imagine that native Python could do it faster thanorjson
could--even accounting for performance penalty ofOPT_NON_STR_KEYS
--and I understood the switch toorjson
to be motivated by performance... The last workaround I can think of is to just never uselog_call
on something that returns adict
whose keys have even a slight chance of not being exactlystr
, but if that were to be the "official" solution I would ask that it at least be documented.The text was updated successfully, but these errors were encountered: