Merge lp://staging/~theiw/txstatsd/txstatsd-fix-871820 into lp://staging/txstatsd

Proposed by Ian Wilkinson
Status: Merged
Approved by: Ian Wilkinson
Approved revision: 47
Merged at revision: 44
Proposed branch: lp://staging/~theiw/txstatsd/txstatsd-fix-871820
Merge into: lp://staging/txstatsd
Diff against target: 244 lines (+109/-17)
4 files modified
txstatsd/server/protocol.py (+53/-4)
txstatsd/service.py (+2/-1)
txstatsd/tests/test_protocol.py (+53/-11)
txstatsd/version.py (+1/-1)
To merge this branch: bzr merge lp://staging/~theiw/txstatsd/txstatsd-fix-871820
Reviewer Review Type Date Requested Status
Lucio Torre (community) Approve
Review via email: mp+79316@code.staging.launchpad.net

Commit message

Provide logging for instances when we are unable to message the Graphite server.

Description of the change

Provide logging for instances when we are unable to message the Graphite server.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

For twisted apps, using twisted.python.log.msg / .err is generally
preferred over python's logging module, isn't it ?

Revision history for this message
Ian Wilkinson (theiw) wrote :

There was a desire not to be prescriptive on logger, in a similar manner to
http://bazaar.launchpad.net/~txstatsd-dev/txstatsd/trunk/view/head:/txstatsd/server/loggingprocessor.py.

On Thursday, 13 October 2011 at 23:52, Robert Collins wrote:

> For twisted apps, using twisted.python.log.msg / .err is generally
> preferred over python's logging module, isn't it ?
>
> --
> https://code.launchpad.net/~theiw/txstatsd/txstatsd-fix-871820/+merge/79316
> You are the owner of lp:~theiw/txstatsd/txstatsd-fix-871820.

Revision history for this message
Lucio Torre (lucio.torre) wrote :

33 + raise TypeError()
should carry a detailed message on the error

87 + reactor.callInThread(self.logger.info, message)
might result in log lines out of order

65 + self.message_graphite_metric.mark(1)
maybe instead of (1) we should mark with the ammount of time it has been paused?

review: Needs Fixing
45. By Ian Wilkinson

Detail tye error, and provide timestamp when composing log message.

46. By Ian Wilkinson

Report the paused period within the GraphiteProtocol.

47. By Ian Wilkinson

Report the total paused period within the GraphiteProtocol. Fixed test now timestamps feature in the logging.

Revision history for this message
Lucio Torre (lucio.torre) wrote :

thanks a lot

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches