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

drop missing keys from KeyValueRender #67

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@sibson
Contributor

sibson commented Apr 25, 2016

Rather than render as None, simply drop any missing keys

sibson added some commits Apr 25, 2016

drop missing keys from KeyValueRender
Rather than render as None, simply drop any missing keys
@codecov-io

This comment has been minimized.

codecov-io commented Apr 25, 2016

Current coverage is 100%

Merging #67 into master will increase coverage by +n/a%

  1. File .../structlog/_utils.py (not in diff) was modified. more
@@           master        #67   diff @@
========================================
  Files          13         13          
  Lines         692        694     +2   
  Methods         0          0          
  Branches       86         88     +2   
========================================
+ Hits          692        694     +2   
  Misses          0          0          
  Partials        0          0          

Powered by Codecov. Last updated by 8df0ef9...d307f3a

@hynek

This comment has been minimized.

Owner

hynek commented Apr 26, 2016

Hmmmm I feel like this should be a separate processor, such that other renderers benefit from it too. Would you agree?

@sibson

This comment has been minimized.

Contributor

sibson commented Apr 26, 2016

Are you suggesting something like

def drop_none_processor(event_dict):
    return dict(k: v for k, v in event_dict if v is not None)

If so the downside is that it would prevent logging None when its a valid value. Otherwise, I don't understand your suggestion.

My particular problem is that KeyValueRenderer is injecting spurious new keys into my log messages when I set a key_order. For example, web requests will have method,path keys which are never present in celery tasks but because Django shares the config I end up with method=None, path=None in my task logs. I could use different config for each but that seems less DRY..

@hynek

This comment has been minimized.

Owner

hynek commented Apr 26, 2016

Ah I see; sorry. I thought you wanted to drop Nones altogether.

# Use an optimized version for each case.
if key_order and sort_keys:
def ordered_items(event_dict):
items = []
for key in key_order:
value = event_dict.pop(key, None)
items.append((key, value))
if value or not drop_missing:

This comment has been minimized.

@hynek

hynek Apr 26, 2016

Owner

This has to be value is not None or else 0 and "" are dropped too.

items += sorted(event_dict.items())
return items
elif key_order:
def ordered_items(event_dict):
items = []
for key in key_order:
value = event_dict.pop(key, None)
items.append((key, value))
if value or not drop_missing:

This comment has been minimized.

@hynek
@sibson

This comment has been minimized.

Contributor

sibson commented May 4, 2016

@hynek updated to fix dropping "" and 0, thanks

@hynek hynek closed this in bc882f4 May 8, 2016

@hynek

This comment has been minimized.

Owner

hynek commented May 8, 2016

thank you!

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