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 MoryxService to handle closing the console window. #357

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

andreniggemann
Copy link
Contributor

  • Add MoryxService to handle closing the console window.
  • Add extension method to KernelServiceCollectionExtensions to use the MoryxService.
  • Add dependency to Microsoft.Extensions.Hosting to make background services available.

@1nf0rmagician
Copy link
Member

The motivation behind this addition is, as I understand, that if you host your MORYX application for example with IIS, everything works fine. But if you want to run the .exe file on your machine, closing the terminal that appears kills the application without reaching the point where the modules are stopped. I.e. this would allow for a graceful shutdown for certain kinds of deployments.

What do you think about it @Toxantron?

@Toxantron
Copy link
Member

Generally I think it is a good idea and one aspect of the old DeveloperConsole that is gone. Right now the shutdown via Ctrl+C should properly shutdown the application.

However: This code is windows only. Since invoking the extension is up to the developer, I don't mind, but maybe there should be some sort of handling to exclude the code on Linux.

@andreniggemann
Copy link
Contributor Author

I talked to @1nf0rmagician about this briefly.
Our idea was to do multitargeting and include this code only with a platform specific framework moniker like net6.0-windows.

Do you agree with this approach?

@Toxantron
Copy link
Member

Toxantron commented Jan 17, 2024

Our idea was to do multitargeting and include this code only with a platform specific framework moniker like net6.0-windows.

Sorry it took so long to respond to this. It simply got forgotten. To be hones @andreniggemann I am not a fan of framework monikers and different versions of DLLs, even if nuget takes care of it. I consider portability a big benefit of .NET and would prefer a runtime check like "if (Environment.OS == Windows)" instead.

And in an additional request I would like to ask for a more descriptive name of the service and extension.

@Toxantron
Copy link
Member

Best practise from Microsoft:

Suggested targets
Use these guidelines to determine which TFM to use in your app:

  • Apps that are portable to multiple platforms should target a base TFM, for example, net8.0. This includes most libraries but also ASP.NET Core and Entity Framework.
  • Platform-specific libraries should target platform-specific flavors. For example, WinForms and WPF projects should target net8.0-windows.
  • Cross-platform application models (Xamarin Forms, ASP.NET Core) and bridge packs (Xamarin Essentials) should at least target the base TFM, for example, net8.0, but might also target additional platform-specific flavors to light-up more APIs or features.

https://learn.microsoft.com/en-us/dotnet/standard/frameworks

@andreniggemann
Copy link
Contributor Author

@Toxantron I will try to see if this is possible with a runtime check around line 66 of MoryxService on Linux.
I don't know what happens when the [DllImport("Kernel32")] Attribute is still there.

I'm not sure about a more descriptive name. I think for other frameworks I have seen it as Add{FrameWork}Hosting.
So maybe calling at AddMoryxHosting instead of AddMoryxService and MoryxHost instead of MoryxService?

To call it something specific to the feature of awaiting the window closing would be weird for compatibility with linux, because it would not do that. Even without the window closing feature the MoryxHost has utility on all platforms in reducing boiler plate in our program.cs files and enabling more general use-cases that rely on the lifecycle of MORYX applications.

@Toxantron
Copy link
Member

@andreniggemann my naming concern mostly comes from the meaning of host and service in the context of ASP and service collection.

Basically everthing you register in the service collection is considered a service and therefor AddMoryxService is quite non descriptive. I like Host, even more something that represents the windows host or window handling.

Add extension method to KernelServiceCollectionExtensions to use the MoryxService.
Add dependency to Microsoft.Extensions.Hosting to make background services available.
Add Platform check, so that it does not crash on other platforms.
Add StateChanged event to enable other servicees to plug into the moryx lifecycle via the host.
@andreniggemann
Copy link
Contributor Author

@Toxantron, @1nf0rmagician reminded me of this topic today.
I finally tested it with the runtime check and it works as expected.
I additionally renamed Service to Host.

Sorry for taking so long it completely slipped my mind.

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

Successfully merging this pull request may close these issues.

3 participants