Fix up pusher logging a bit #4716
Conversation
|
(attempts to) fix #4710 I guess |
| @@ -97,6 +97,11 @@ def __init__(self, hs, pusherdict): | |||
| pusherdict['pushkey'], | |||
| ) | |||
|
|
|||
| if self.data is None: | |||
| raise PusherConfigException( | |||
There was a problem hiding this comment.
will we actually log any less for a PusherConfigException than for a TypeError or whatever it was?
There was a problem hiding this comment.
Woops, I missed a code path where it wasn't handled.
| @@ -56,7 +56,7 @@ def create_pusher(self, pusherdict): | |||
| f = self.pusher_types.get(kind, None) | |||
| if not f: | |||
| return None | |||
| logger.info("creating %s pusher for %r", kind, pusherdict) | |||
| logger.debug("creating %s pusher for %r", kind, pusherdict) | |||
There was a problem hiding this comment.
I'm not entirely convinced by this. it's the only thing that gets logged during minutes of cpu-spinning, and it's not massively high-volume.
There was a problem hiding this comment.
It's logging several times a millisecond with a reasonably a non-trivial dict, which feels a bit OTT. There are ~170K pushers, with a log length of ~600 characters, means about 102MB of logs
I could make it so it logs in batches the progress?
There was a problem hiding this comment.
fair enough. let's see how it looks.
Codecov Report
@@ Coverage Diff @@
## develop #4716 +/- ##
===========================================
- Coverage 75.14% 75.03% -0.12%
===========================================
Files 340 340
Lines 34815 34822 +7
Branches 5702 5704 +2
===========================================
- Hits 26162 26127 -35
- Misses 7044 7074 +30
- Partials 1609 1621 +12 |
| pusherdict.get('user_name'), | ||
| pusherdict.get('app_id'), | ||
| pusherdict.get('pushkey'), | ||
| e, |
There was a problem hiding this comment.
I think you generally need the type of the exception as well as the stringification
(mostly because KeyErrors are otherwise extremely confusing:
>>> try:
... {}['a']
... except Exception as e:
... print(e)
...
'a'
)
There was a problem hiding this comment.
Err, we know the type since we're only catching the one PusherConfigException exception?
| @@ -56,7 +56,7 @@ def create_pusher(self, pusherdict): | |||
| f = self.pusher_types.get(kind, None) | |||
| if not f: | |||
| return None | |||
| logger.info("creating %s pusher for %r", kind, pusherdict) | |||
| logger.debug("creating %s pusher for %r", kind, pusherdict) | |||
There was a problem hiding this comment.
fair enough. let's see how it looks.
No description provided.