-
Notifications
You must be signed in to change notification settings - Fork 84
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 possibility for caller proxy to return an observable #330
base: master
Are you sure you want to change the base?
Conversation
Also I was wondering which C# language features I could use... Is C# 7 or even 8 allowed? |
C# 7.0 is okay. C# 8.0 is not supported for .NET Framework, so let's avoid it (I hope I have been doing that myself). |
src/netstandard/WampSharp/WAMP2/V2/Api/CalleeProxy/ObservableCalleeProxyInterceptor.cs
Outdated
Show resolved
Hide resolved
src/netstandard/WampSharp/WAMP2/V2/Api/CalleeProxy/CalleeProxyInterceptor.cs
Outdated
Show resolved
Hide resolved
77226d9
to
2c91ef6
Compare
When subscribed to it will call a progressive RPC Code-Sharp#238
As discussed in the review, this extra declaration is a requirement to make sure that people really want the method to be progressive and not return a IObservable derived type by accident. Code-Sharp#238
Hi @darkl I think I have most of it, if not everything, in now to support IObservable return values on the callee side as well as on the caller side. Can you please take another look at it and show me where I might have messed up? :) |
Thanks @Jopie64 for your work! |
I am sorry this is taking so long, I am busy with other stuff. One thing that bothers me is that the final value will always be empty. This causes inconsistencies because if you have an old implementation using Elad |
Yea I feel your pain, there's always lots of important stuff to do for us devs. No hurries 😊 About that final value thing. That bothered me too for a while. But in practice it doesn't really matter at least for us. The thing to keep in mind is: It is not a 100% replacement for the IProgress API.
In all other cases, where you just have an observable implementation on a different node than the observable subscription, this pattern can be used. For us in practice it means that we never had to make use of the extra capabilities the IProgress interface gives us. (We are currently using a workaround to be able to use this pattern.) |
I think I want to change the behavior so that the CalleeProxy will omit the return value unless specified explicitly (maybe through an attribute or something). Elad |
I don't understand what you mean... I assume you mean the final return value? In case of observables the callee already omits its final return value right? Because It's the caller that you might consider confusing I think. In WAMP the final return might carry a value or it might not. In case of observables you don't expect the final result to contain a value. But I currently made it so that when it does anyway, it will emit this before it completes. The reason I did it this way is that this way the caller can use observable return values in all methods, where the server can implement them as observable or task. In practice this works well for us. The caller (even when it is mostly javascript) uses observables everywhere and the server backend implements it however it wants. And an existing API can even switch from task to observable without requirering a change in the frontend. |
First part of #238.
Can you take a look at it for whether I'm on the good track with this feature?