Code review comment for lp:~thumper/juju-core/fix-backlog-limits

Revision history for this message
Roger Peppe (rogpeppe) wrote :

> Due to the buffered i/o in the tailer, I'd prefer to keep the line
parsing
> functionality in the server side.

I don't quite understand this remark (the buffer is flushed whenever we
get to the end of the file, so it shouldn't make any significant
difference), but splitting out the backtracking code makes me much
happier.

LGTM with a couple of trivial suggestions below.

https://codereview.appspot.com/85570045/diff/20001/state/apiserver/debuglog.go
File state/apiserver/debuglog.go (right):

https://codereview.appspot.com/85570045/diff/20001/state/apiserver/debuglog.go#newcode224
state/apiserver/debuglog.go:224: if !stream.fromTheStart {
if stream.fromTheStart {
     return nil
}
...

saves a negative and a level of indentation.

https://codereview.appspot.com/85570045/diff/20001/state/apiserver/debuglog.go#newcode258
state/apiserver/debuglog.go:258: if stream.started && result &&
stream.maxLines > 0 {
On 2014/04/14 03:52:40, axw wrote:
> It would be better to just split filterLine into something
non-counting and
> counting, where the latter calls the former and adds the linecount
check. Then
> you don't need the boolean.

+1

https://codereview.appspot.com/85570045/diff/20001/utils/tailer/tailer.go
File utils/tailer/tailer.go (right):

https://codereview.appspot.com/85570045/diff/20001/utils/tailer/tailer.go#newcode31
utils/tailer/tailer.go:31: // TailerFilterStartedFunc is a callback that
is called when the filtering is
On 2014/04/14 03:52:40, axw wrote:
> Delete this?

+1

https://codereview.appspot.com/85570045/diff/20001/utils/tailer/tailer.go#newcode132
utils/tailer/tailer.go:132: // wanted number of filtered lines before
the end.
// If filter is non-nil, only lines for which filter returns
// true will be counted.

?

https://codereview.appspot.com/85570045/

« Back to merge proposal