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

Add rpc::future for callback support to client async calls #45

Open
sztomi opened this issue Feb 15, 2016 · 10 comments
Open

Add rpc::future for callback support to client async calls #45

sztomi opened this issue Feb 15, 2016 · 10 comments
Assignees
Milestone

Comments

@sztomi
Copy link
Collaborator

sztomi commented Feb 15, 2016

No description provided.

@sztomi sztomi self-assigned this Feb 15, 2016
@sztomi sztomi added this to the v2.0.0 milestone Feb 15, 2016
@ahoarau
Copy link
Contributor

ahoarau commented Jul 31, 2017

Would you consider having rpclib::future instead of std::future ? I read that .then() operator is not gonna be in c++ for a while.

@ahoarau
Copy link
Contributor

ahoarau commented Jul 31, 2017

The folly library from facebook implements them, but I guess this dependency is way too big.

@sztomi
Copy link
Collaborator Author

sztomi commented Nov 6, 2017

And sorry, it seems I never replied to this: yes, I'd consider it, absolutely.

@sztomi
Copy link
Collaborator Author

sztomi commented Nov 6, 2017

So my thinking is: providing rpc::future is one way to solve this, but it would also be possible to do it without that. For example, async_call could have an overloaded signature like:

std::future<RPCLIB_MSGPACK::object_handl> async_call(invocation i, callback c);

And used like:

client.async_call({"hello, 2, 3}, [](something) { ... });

Obviously, rpc::future would look smoother but that's paid for by additional complexity in the library. That is not say I'm against it, but I do want to see what options are there.

BTW I'm doing some breaking changes for 3.0, so pretty much anything goes for now in terms of interface breakage.

@ahoarau
Copy link
Contributor

ahoarau commented Nov 6, 2017

Excellent. I really look forward for this feature.

For now I'm using this hack in Qt :

    connect(ui->actionCalibrate,&QAction::triggered,this,[=](){

        auto f = client->async_call("calibrate");

        QFuture<void> future = QtConcurrent::run([this,&f](){
            bool done = f.get().as<bool>();
        });
    
    });

@sztomi
Copy link
Collaborator Author

sztomi commented Nov 6, 2017

What do you think about the proposed overload?

@ahoarau
Copy link
Contributor

ahoarau commented Nov 6, 2017

That would work, but I think that would be clearer to have :

auto f = client->async_call("add",2,3);
auto f = client->async_call("add",2,3).then( [](){} );

Rather than :

auto f = client->async_call("add",2,3);
auto f = client->async_call( {"add",2,3} , [](){} ); // is that you'd construct the invoker ?

@sztomi
Copy link
Collaborator Author

sztomi commented Nov 6, 2017

Yeah, that's pretty much my proposal.

I agree that .then is probably better, but it's also a bit more complicated to implement. I wonder if there is a good C++11 implementation of it which is a lot like std::future but with then

@ahoarau
Copy link
Contributor

ahoarau commented Nov 6, 2017

I think you can take from https://github.com/jaredhoberock/future, it seems promising 👍

@sztomi
Copy link
Collaborator Author

sztomi commented Nov 6, 2017

If I understand correctly, that is a third option: then as a free function.

But I also found that boost::future can provide a then function. So I'll probably use that.

@sztomi sztomi added the ready label Nov 21, 2017
@sztomi sztomi modified the milestones: v2.1.0, v3.0.0 Nov 21, 2017
@sztomi sztomi changed the title Add callback support to client async calls Add rpc::future for callback support to client async calls Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants