-
Notifications
You must be signed in to change notification settings - Fork 102
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
Stop Microsoft.Azure.SignalR.Management including a FrameworkReference for Microsoft.AspNetCore.App #1924
Comments
|
My understanding is that you don't have to reference the framework, you can refernce the individual NuGet packages for the components you need. By not demanding the framework and instead referencing the NuGet packages directly the consumers of Microsoft.Azure.SignalR.Management hosting in a Microsoft.AspNetCore.App project will automatically ignore the NuGet packages. Customers hosting in a WindowsDesktop app (or other) will continue to reference the NuGet packages and the deployment will work. The problem that exists today is that if I add the NuGet to a Microsoft.AspNetCore.App project everything works. It is reasonably expected that the AspNetCore runtime is required on the host. If I add the NuGet to any other framework project everything runs on a developer machine and compiles, but when it is deployed to a host you get strange errors about not being able to find an assembly and it is not immediately obvious that you must deploy the AspNetCore runtime for a Windows Service or Console app. I think it worth point out that if you add the NuGet to a .Net Framework 4.7.2 project that it does reference the AspNetCore NuGet packages and it does include them in the build output and it does all work on a host that does not have the AspNetCore runtime deployed. |
Since .NET Core 3.1, SignalR doesn't have a server nuget package anymore. Included in the Microsoft.AspNetCore.App shared framework. See https://learn.microsoft.com/en-us/aspnet/core/signalr/version-differences?view=aspnetcore-8.0#how-to-identify-the-signalr-version |
The change I'm suggesting isn't to re-create a server nuget; although that may be a good idea. My suggestion is that be referencing the required NuGet packages directly instead of including an AspNet Framework reference then you allow for simpler deployment. If the application includes the AspNetFramework reference then any NuGet Package (SignalR) that references the AspNet NuGet packages will auto-upgrade/ignore the package references. If the application does not (for instance a console app) then it will include the NuGet references and run on a host that does not have the AspNet Runtime deployed. For example, the sample project (https://github.com/Azure/azure-signalr/tree/dev/samples/ChatSample/ChatSample.CSharpClient). If upgraded to .net 8.0 then the host executing it must have the AspNet Runtime installed, even though the consumer thinks they are running a console application. If the SignalR packages just referenced the required AspNet NuGet packages then in this deployment scenario the AspNet dependencies would ship with the application itself. However the AspNet samples that include the framework reference (such as an upgraded to .net 8 https://github.com/Azure/azure-signalr/tree/dev/samples/ChatSample/ChatSample.Net70) would not trigger the AspNet dependencies to ship, because they are known to already be present on any host, because the whole application depends on the AspNet Runtime. It is basically strange that a transient dependency of my console application depends on a specific runtime, and Visual Studio / Build Pipelines do not seem to gracefully handle this situation - maybe it should be blocked... if a transient dependency demands the AspNet runtime then maybe the application itself must demand that to avoid confusion - obviously that is not a debate for this repo though. |
I understand your concern, but your solution is not realistic.
There is no single SignalR NuGet package for us to refer in .NET 8. To use interfaces like Asp.NET Core SignalR and Asp.NET SignalR are different things. And to use SignalR in .NET 8 ( .NET 8 is also a Asp,NET Core vesion), you have to refer to the .NET 8 SignalR, instead of Asp.NET Core 2.0 SignalR, or Asp.NET SignalR. |
Gotcha. I understand what you mean now. And as there is no Microsoft.AspNetCore.App package you can't reference them another way. I do still think this is an issue - I appreciate the difficulty in resolution, but the decision to bundle SignalR into the core AspNetCore codebase is really saying to the world "SignalR is only for use in AspNetCore applications" - which I don't believe is true - there is a code sample for a console app which is pretty similar to our use-case which involves some Asp.Net projects and some Windows Services - which run on different hosts, and only the ones running sites have the AspNet Runtime deployed at present. The Console App should certainly be updated to .net 8 and some big warnings to flag that running it require the AspNetCore runtime to be deployed. Given the migration of code into the AspNetCore repository I'm a tad confused regarding the governance/structure? Why does this repo exist when some of the SignalR code is elsewhere now? Feels strange at best. Is there a roadmap that more of this is moving into the core Sdk codebase, or that the management NuGet will drop the dependency? |
Maybe the team responsible for the core Sdk should be moving the SignalR functionality into the core Sdk rather than the AspNetCore one? |
cc @davidfowl Is it possible to extract common abstractions like |
If you don't want to install the ASP.NET Core runtime on your host machine, then publish your app as self-contained. https://learn.microsoft.com/dotnet/core/deploying/#publish-self-contained |
I did look into this but it isn't possible. Our "app" is really a plug-in for another app. So the way we've had to make it work is to modify the apps runtimeconfig.json to include the Microsoft.AspNetCore.App framework, and deploy said framework to the host. Along with this being a frustrating requirement in the Management package it also feels like the Sdk/MSBuild targets/Visual Studio don't handle framework dependencies in transient NuGet packages well. I appreciate that plugin development isn't the norm, but even if our "app" were an Exe and teh final target, there are no warnings or prompts in Visual Studio or builds to suggest "Nuget Package XXX depends on a targetframework yyy which is not referenced". The build pipeline handles it automatically by auto-injecting the framework into the runtimeconfig.json file, but because the developer isn't told about the requirement they dont know their hosting requirements have changed. And so it works on the developer machine with the full sdk deployed but when it is deployed to a host without the asp runtime pack you get a strange Missing File exceptions. The cause does not easily/quickly map to the symptom. Obviously that is a criticism of the core sdk - in my opinion it should not automatically handle it, raising a build error and forcing the developer to be aware that they need to add a framework reference to their project would be better. |
Is your feature request related to a problem? Please describe.
The Microsoft.Azure.SignalR.Management includes a FrameworkReference for Microsoft.AspNetCore.App even though a common use-case would be to broadcast messages from a service application; which is even called out in the project readme.
This causes apps targetting the Microsoft.Windows.SDK.NET.Ref framework to require the AspNet Core runtime to be deployed on the host to function.
When referencing the package from .Net Framework 4.7.2 all the dependencies are deployed.
When referencing the package from .net 8.0 any dependency that is included with the Asp Net Core runtime is excluded.
Describe the solution you'd like
Microsoft.Azure.SignalR.Management should only have a framework dependency of Microsoft.NETCore.App and should avoid references to AspNet to support basic functions regarding broadcasting messages, checking connected users, etc.
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: