nats-io/nats.rs

Expose internal command `mpsc::channel` in public API

rvolosatovs opened this issue · 4 comments

Proposed change

I'd like to expose

pub(crate) sender: mpsc::Sender<Command>,
as a Sink<async_nats::Command> in some way (or even just a plain channel) - see use case below

This could look like

fn command_sink(&self) -> impl Sink<async_nats::Command>

or simply

fn command_sender(&self) -> mpsc::Sender<async_nats::Command>

This would require making async_nats::Command public

This would be an advanced functionality and I'm happy to e.g. guard this by an opt-in feature flag if there are concerns around that

The sender -> sink mapping itself is facilitated by https://docs.rs/tokio-util/latest/tokio_util/sync/struct.PollSender.html#impl-Sink%3CT%3E-for-PollSender%3CT%3E

Use case

I'm using NATS as a byte stream transport and to facilitate that I'd like to implement a Sink pairing a client and a subject. Unfortunately, plain publish and related functionality does not provide enough data to be able to do that correctly.

Contribution

yes, will do so in a few hours

Hey!

Thank you for your proposal and suggestion!

I understand that use case and having a nice way to use sink would be good, however I do not like exposing Sender, as this exposes internal API and prevents us from refactoring the internals without breaking users. And we do plan to change those internals at some point in time, depending how tokio evolution, its mpsc and other factors play out.

As usual I'm against any unnecessary layers of indirection, in this case, I would prefer having one.

Hey!

Thank you for your proposal and suggestion!

I understand that use case and having a nice way to use sink would be good, however I do not like exposing Sender, as this exposes internal API and prevents us from refactoring the internals without breaking users. And we do plan to change those internals at some point in time, depending how tokio evolution, its mpsc and other factors play out.

As usual I'm against any unnecessary layers of indirection, in this case, I would prefer having one.

That's understandable, so how about the impl Sink suggestion?

In fact, it could be a publish_sink(&self, subject: impl ToSubject) -> impl Sink<Bytes>, so it would not even require exposing the Command

I've reworked #1267 and added 5354f82, which now adds a Publisher struct, which is essentially the opposite side of a Subscriber, which already exists.

WDYT?

@Jarema Looking at #1267, it looks like it is now pretty clean and straightforward as it doesn't expose the internals now