Replace the `logger *log.Logger` dependence with interface

[IhsanMujdeci]

I am trying to use your application through importing it from golang. The way the code works is that it requires a log.Logger then uses the log.Printf and log.Print function from this struct. If we could replace the log.Logger with lets say a "Printer" interface which contains only Printf and Print then users can write their own and have the value saved to a struct or something else.

[yannh]

That might work! The trick is that you might get many simultaneous calls, and simple printfs writing all at the same time wouldnt work well. See https://newbedev.com/when-to-use-log-over-fmt-for-debugging-and-printing-error

Redis-dump-go relies on this behaviour, just exposing the interface without serialising the calls might be pretty dangerous. What do you think?

[IhsanMujdeci]

Okay I worked around this. I create my own log.Logger with log.New.

type byteWriter struct {
    B *bytes.Buffer
}

func (w byteWriter) Write(p []byte) (n int, err error) {
    w.B.Write(p)
    return len(p), nil
}

func main(){
    var s = new(bytes.Buffer)
    b := byteWriter{s}
    logger := log.New(b, "", 0)
}

I then plugged this logger into your DumpDb to get a byte array then I wrote it to a file :) Do you see any gaps with this?

[yannh]

Just be sure that it's concurrency-save, and that multiple writers at the same time are safe and will not produce garbage output! Note: I ve not heard of people using this as a library in the past, so the interface are... maybe not super clean, nor set in stone.

Which function are you using, DumpServer? Might be worth revisiting what would be worth exposing. Would love your input, especially around logging.

[IhsanMujdeci]

Im using dump db. I have single instance redis cluster. I THINK my approach is thread safe. I'm a bit of a golang noob so I'm not sure. The thing is that log.New has mutexes and concurrency safe stuff in the background. What this logger does that instead of logging to std.Out it logs to a buffer.

Tips for making your api consumption more friendly is make params default to some value as they do on cli and please make an easy way to handle / export that progress channel.

Also if you deem my solution suitable and thread safe suggest it as a way to use this api.

[yannh]

I agree I think that should work :) Using log.SetOutput to a custom io.Writer should also do the trick. That should give enough flexibility while guaranteeing to some degree the thread safety. I'll document this at some point. Thanks!