Don't unsafely run effect from Producer's send callback

[RibleStrype]

Even though semantically the behaviour is the same, I think it is a bit cleaner when we don't "unsafely" run the effect (completing Deferred) via ToTry in Producer#send callback.

Also as the docs state the Callback is generally executed in the I/O thread of the producer, so it should be as fast as possible. To me it seems that completing a Promise is more lightweight than executing an effect of completing Deferred.

[coveralls]

Pull Request Test Coverage Report for Build 1841127528

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• 33 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-1.8%) to 83.129%

---

Files with Coverage Reduction	New Missed Lines	%
skafka/src/main/scala/com/evolutiongaming/skafka/consumer/ConsumerConverters.scala	1	95.08%
skafka/src/main/scala/com/evolutiongaming/skafka/Converters.scala	1	89.39%
skafka/src/main/scala/com/evolutiongaming/skafka/producer/ProducerLogging.scala	1	52.94%
skafka/src/main/scala/com/evolutiongaming/skafka/consumer/ConsumerLogging.scala	30	68.93%

Totals
Change from base Build 1751109306:	-1.8%
Covered Lines:	1291
Relevant Lines:	1553

---

💛 - Coveralls

[t3hnar]

why do you think that this is anyhow faster?

[RibleStrype]

I haven't measured it of course, it just intuitively feels more lightweight, but I don't have any strong arguments in this regard.

[t3hnar]

I'd recommend keep using deferred but call toFuture instead of toTry and deprecate API requiring ToTry

[RibleStrype]

But how is ToFuture better than ToTry, don't they both call unsafeRunSync in implementation?

[t3hnar]

But how is ToFuture better than ToTry, don't they both call unsafeRunSync in implementation?

1. less construction requirements
2. ToFuture uses unsafeToFuture, and truly saying both are ok in this case

[RibleStrype]

So if I change ToTry to ToFuture what do I do with the resulting Future? Do I await inside the Callback or just ignore it? TBH I don't like any of these options and would rather leave ToTry then.
btw, what's the reason you are against using Promise here? To me it seems cleaner that materialising an effect in any manner

[t3hnar]

Do I await inside the Callback or just ignore it?

Just ignore it, as executed code is private and you exactly know what is being executed

[t3hnar]

the reason you are against using Promise here

Because that combination of Promise/Future adds more overhead than you might think of comparing to native, it is also not a cancellable thing.

[RibleStrype]

But this particular use case is not cancellable either way, is it? Even if you cancel the resulting effect the producer's callback will still be executed

[RibleStrype]

I mean cancelling the "inner" effect resulting from Deferred#get

[t3hnar]

breaks backward compatibility

[RibleStrype]

I though it might break it, but MiMa checks passing kinda convinced me otherwise.

[RibleStrype]

Or do you mean source compatibility?

[t3hnar]

breaks backward compatibility

[RibleStrype]

But how does MiMa check pass then?

[t3hnar]

But how does MiMa check pass then?

good question, should not pass