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

Signalr Extensions using Class-based model is not currently supported #771

Open
michaeldaw opened this issue Jan 8, 2022 · 17 comments
Open
Assignees

Comments

@michaeldaw
Copy link

Worker.Extensions.SignalRService does not currently contains classes required to develop Functions using the Class based model.
eg:

  • ServerlessHub
  • InvocationContext
  • SignalRConnectionInfo

Are there plans to support the Class Based Model when using Worker.Extensions.SignalRService?

@fabiocav
Copy link
Member

@Y-Sindo have you had a chance to look at this? Some of them should be very simple to bring in.

@Y-Sindo
Copy link
Member

Y-Sindo commented Jan 12, 2022

I will add a PR to provide some convenient classes such as SignalRConnectionInfo, SignalRInvocationContext soon.

About the serverless hub in isolated-process model, we do have a plan to support it, but we still have some problems to overcome.

@Y-Sindo
Copy link
Member

Y-Sindo commented Jan 12, 2022

@fabiocav I have added a PR to address the issue, please take a look at it.

@fabiocav
Copy link
Member

Fantastic! Thanks @Y-Sindo. We'll review that ASAP.

FYI @anthonychu

@michaeldaw
Copy link
Author

Sorry to interrupt the discussion. Just wanted to say thanks for the response.

@Arash-Sabet
Copy link

@fabiocav @anthonychu Any idea when the PR associated with this issue will be approved and released? Thanks.

@Y-Sindo
Copy link
Member

Y-Sindo commented Feb 18, 2022

@fabiocav @kshyju Currently the SignalR class-based model can bind the RPC call parameters to trigger function parameters automatically, I want to know if the bindings in host process can get the parameter info of the trigger functions in worker process, so that we can do similar things in isolated model.

I also have other related questions in #806 .

@liliankasem
Copy link
Member

@kshyju looks like the PR was merged last week, any idea on a potential release date?

@michaeldaw
Copy link
Author

Thanks to everyone who's worked on this. Will it be part of a release soon? I've started a project that could really use this 😊

@Y-Sindo
Copy link
Member

Y-Sindo commented Apr 4, 2022

In the latest version 1.7.0, we have provided some built-in POJOs such as SignalRConnectionInfo and SignalRInvocationContext. I will provide a sample later.

For class-based SignalR trigger, it's still in the back log.

@duncanthescot
Copy link

What is the current status of this? The last message I see is from a year ago.

@Y-Sindo
Copy link
Member

Y-Sindo commented Apr 21, 2023

The class-based model can be broken down into two main tasks. The first task involves resolving SignalR method names and arguments directly from the function signatures, without the need for them to be specified in the SignalRTriggerAttribute parameters. However, it's unclear whether we can obtain the function signatures in the host process. If we can, then this task can be easily accomplished. If not, we'll need to rewrite the entire SignalR trigger logic in the worker process. Unfortunately, during my investigation, I encountered a blocker #1479 which has halted progress on this front. We'll have to wait until this issue is resolved before we can move forward.

The second task is related to SignalR negotiation and message sending. This involves using an SDK-style client instead of input binding and output binding. This task is similar to the preview feature known as the SDK types binding in dotnet isolated worker, which is currently in an early stage and not yet supported by SignalR. In the meantime, a possible workaround could be to use the Azure SignalR management SDK . To make the integration with the management SDK easier, I'm currently working on writing a sample and exploring other possible solutions.

@Y-Sindo
Copy link
Member

Y-Sindo commented Apr 23, 2023

Please see this project for an example to integrate with SignalR management SDK https://github.com/aspnet/AzureSignalR-samples/tree/2e928b19f545b4d66c943569c21b3ef296b0397b/samples/DotnetIsolated-ClassBased

@Y-Sindo
Copy link
Member

Y-Sindo commented Apr 27, 2023

@fabiocav
Edit: One of the benefits of class-based model is that we don't need to specify the SignalR invocation parameter names but still get them bound. The following code snippets are a comparison of non-class-based model and class-based model.

// non-class-based model
[FunctionName("Broadcast")]
public Task Broadcast(
    [SignalRTrigger("Broadcast", "messages", "SendMessage", parameterNames: new string[] {"signalRRemoteInvocationParameter1", "signalRRemoteInvocationParameter2"})]InvocationContext invocationContext, 
    string signalRRemoteInvocationParameter1, 
    string signalRRemoteInvocationParameter2) { }

// class-based model
[Function("Broadcast")]
public Task Broadcast(
    [SignalRTrigger] SignalRInvocationContext invocationContext, 
    string signalRRemoteInvocationParameter1, 
    string signalRRemoteInvocationParameter2) { }

We achieve this in the in-process library by resolving the parameters to bind from the MethodInfo of the function. However, in the case of isolated workers, the host process is unable to obtain the actual MethodInfo of functions within isolated workers; in my test, the method.GetParameters() returns 5 parameters which are JObject invocationContext, IBinder _binder, ExceutionContext _context, ILogger _logger, _cancellationToken. Therefore, we cannot determine the ITriggerBinding.BindingDataContract for a SignalR trigger beforehand without declaring the parameters in the trigger.

Given this, can we get the parameters bound without declaring them in the trigger attribute?

@michaeldaw
Copy link
Author

michaeldaw commented Apr 27, 2023

@Y-Sindo If we're still talking about non-class-based mode, I've had to declare the function trigger as in the example below in order to ensure the parameters are correctly bound:

    [Function("MessageFromClient")]
    public async Task ClientCommand(
        [SignalRTrigger("hub", "messages", "MessageFromClient", "parameter1", "parameter2")]
        SignalRInvocationContext invocationContext, string parameter1, string parameter2)
    {
    }

To be clear, this is for a trigger in which a message is coming from the client to the server. The parameters of the SignalRTrigger attribute are "hubName", "category", "eventName", followed by a params list of parameter names.

Edit:
One thing I have not been able to do successfully is return a value to the client from a trigger like the above. In earlier projects (before I switched to Isolated-mode), I was able to specify a return value for the function. The return value would be serialized and sent back to the client. Since using isolated mode, I've found that returns values are null/undefined when they get to the client.

@michaeldaw
Copy link
Author

In any case, since creating this issue I've learned to live with the non-class-based mode 😅.

@Y-Sindo
Copy link
Member

Y-Sindo commented Apr 27, 2023

@michaeldaw

@Y-Sindo If we're still talking about non-class-based mode, I've had to declare the function trigger as in the example below in order to ensure the parameters are correctly bound:

    [Function("MessageFromClient")]
    public async Task ClientCommand(
        [SignalRTrigger("hub", "messages", "MessageFromClient", "parameter1", "parameter2")]
        SignalRInvocationContext invocationContext, string parameter1, string parameter2)
    {
    }

To be clear, this is for a trigger in which a message is coming from the client to the server. The parameters of the SignalRTrigger attribute are "hubName", "category", "eventName", followed by a params list of parameter names.

Yes, you're right. To remove the requirement of declaring the method name and parameter names is one of the tasks in serverless hub. Currently I've encountered an issue when trying to implement it. See my earlier comment for detail: #771 (comment)

Edit: One thing I have not been able to do successfully is return a value to the client from a trigger like the above. In earlier projects (before I switched to Isolated-mode), I was able to specify a return value for the function. The return value would be serialized and sent back to the client. Since using isolated mode, I've found that returns values are null/undefined when they get to the client.

Confirmed the problem. I have created a new issue #1496 to check the problem. It still needs more investigation.

Thank you for sharing your experience. We appreciate your adaptability in working with the non-class-based mode. If you have any further issues or suggestions, please feel free to let us know. We value your feedback as we continue to improve our software.

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

No branches or pull requests

8 participants