Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose a blocking module in async_nats #1272

Open
cortopy opened this issue May 27, 2024 · 4 comments
Open

Expose a blocking module in async_nats #1272

cortopy opened this issue May 27, 2024 · 4 comments
Labels
proposal Enhancement idea or proposal

Comments

@cortopy
Copy link

cortopy commented May 27, 2024

Proposed change

As per the main README, the old blocking nats create is considered legacy, and it's recommended to use async_nats now.

This is the reverse of what the Rust community had a few years ago, when nats was the default and a wrapper was given for async routines. There were a lot of discussion at the time in this repo about the problems this approach brought. I believe that's where async_nats was born.

While this approach brings this crate maintenance in line with what other crates do, a maintained blocking option would be great. Consider for example reqwest, which is async by default and then has a wrapper for blocking.

Use case

I use nats crates in both async and blocking routines. While maintaining both is too much, as a user I still need to use nats in both contexts.

Contribution

No response

@cortopy cortopy added the proposal Enhancement idea or proposal label May 27, 2024
@Jarema
Copy link
Member

Jarema commented May 27, 2024

Hey.
We did consider a blocking abstraction, however, considering how simple it is to do it manually (and how more flexible and less opinionated it becomes) I was wondering if its a good idea.

Take a look at the example from examples/sync_context:

use futures::StreamExt;

fn main() -> Result<(), async_nats::Error> {
    // Spawn a new runtime that will run on the current thread.
    let rt = tokio::runtime::Builder::new_current_thread()
        .enable_all()
        .build()?;

    // We are using `block_on` to run the async code in a sync context.
    let client = rt.block_on(async_nats::connect("nats://localhost:4222"))?;

    // Subscribe a client to the "foo" subject.
    let subscription = rt.block_on(client.subscribe("foo"))?;

    // Limit the number of messages to 10. This does not have to be done inside the
    // async context.
    let mut subscription = subscription.take(10);

    // Publish some messages
    for _ in 0..10 {
        rt.block_on(client.publish("foo", "Hello, sync code!".into()))?;
    }

    // To receive message in a loop, you can use the same pattern as you would in an async context.
    while let Some(message) = rt.block_on(subscription.next()) {
        println!("Received message {:?}", message);
    }

    // You need to drop subscripions in async context, as they do spawn tasks to clean themselves up.
    rt.block_on(async {
        drop(subscription);
        drop(client);
    });

    Ok(())
}

This allows you to have full control over how much work you want to do in async context before switching back, how you want to configure runtime, etc.

EDIT: There are also some edge cases like dropping subscriptions which are caused by lack of async Drop trait that might make making a nice and easy wrapper a more painful journey.

Considering how simple it is, do you stil think it's worth pursuing a wrapper for whole client?

@cortopy
Copy link
Author

cortopy commented May 27, 2024

thanks @Jarema for such a quick response.

I had assumed it would be easy like that. But I use nats in several crates, and this would force me to create my own crate so that I can share the code between them. And even in this case, there are edge cases like you mention that I'm not going to be aware of.

What concerns me though is that by exposing a blocking module, users can report any unexpected issues that may arise. It may look simple but there may be scenarios that require battle testing. Only with the right user input I'd have confidence it works. This is the main reason I carry on using the old nats crate in addtion to async_nats. It may not have all the functionality but I know it's tested and reliable.

IMHO, the ultimate example is reqwest. It's just so easy to go to the documentation and check everything there, even if it's a thin wrapper. But even so the concept of "thin" is relative. Looking at all the files that are part of the blocking module makes me realise that all users of async_nats wanting to use it as blocking would need to write a lot of boilerplate each time.

@Jarema
Copy link
Member

Jarema commented May 27, 2024

Hey.
I understand your point.

However I think that rewquest and async-nats are that close to each other:

  • nats clients create a long lasting connection with protocol support.
  • nats is in nature asyncrhonous communication, meaning there might be many things happening at the same time and different use cases may require different approach to how you block.

I'm not saying no. Just bringing some light why its pretty simple to do it for a specific use case, but not that simple as a general solution.

We can definately discuss this and have some ideas here.

@caspervonb
Copy link
Collaborator

The long term end goal here (for all of rust) would probably be that rust moves ahead with keyword generics or a similar initiative.

See:
https://blog.rust-lang.org/inside-rust/2022/07/27/keyword-generics.html
https://www.youtube.com/watch?v=MTnIexTt9Dk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Enhancement idea or proposal
Projects
None yet
Development

No branches or pull requests

3 participants