feature: Install remove instances #283
Conversation
RemainAfterExit=yes makes impossible for service to restart automatically if exited with 0 exit code
a bit messy commit, might require more polishing
|
Hi thanks a lot for this PR, i will have a look at it. That feature was on my todo list for a long time, i just haven't had time to start it myself. For the streamer installation, please make a separate PR. Im likely going to push a commit today which will hide the install option from the install menu temporarily. Too many "bug-reports" from people not understanding the meaning of the "function currently disabled" message. |
|
@CODeRUS If you are ok with it I would do an PR containing Crowsnest install. I dont know if it would be useful to fix outdated mjpeg streamer, but imho not. |
|
@th33xitus okay for streamer. For same technique i meant: choose klipper instance to determine "printer_data" folder to put streamer config to, then ask for name. |
|
@KwadFan crownest should be easy, it contains own installation scripts asking users for details |
|
okay then, just waiting for reviews :) |
I think so, especially for me, I am the creator of ;) The question was more or less, will you skip that step then? Dont want to do work twice or better said made by different peops... |
|
@KwadFan sure, go ahead, it would be good if you can implement it for kiauh. |
|
Great, for sure will do during this week. |
|
@CODeRUS sorry that i still haven't reviewed. Im currently in the process of moving house to a different city, so i have very very little time currently, i hope i will find some time during the next weeks. i hope thats okay. just wanted to let you know that i havent forgot about this PR. |
|
I tested this out and it appeared to work. The interface confused me at first but I figured it out. I had two instances installed from my original install and wanted to add two more. The sequence that worked was to go to Klipper install and put 4. Then enter the same names as the original instances and allow it to warn that they already exist. Then enter the two new names and it installed Klipper for all 4 instances. Next I went to moonraker install and it showed the Klipper instances that wasn't installed for and I was able to install for them. Even though it was a little confusing I'm happy I was able to do this without starting over. |
|
@randallknutson thanks for testing! I agree this may require some nice touches by maintainer itself to ake it ore clear for user and better fit to existing design. |
|
@th33xitus fyi: updated to master, with fixing couple small things i spotted |
|
@CODeRUS thank you, but in case you find more such things, please don't push them into this PR and make new ones if they can be applied standalone to master. lets keep this PR only for the requested feature, that will make things much easier. im going to start pushing to this PR as well from now on to finally start and get this feature in. hope you are okay with this. |
Signed-off-by: Dominik Willner <[email protected]>
Signed-off-by: Dominik Willner <[email protected]>
Signed-off-by: Dominik Willner <[email protected]>
Signed-off-by: Dominik Willner <[email protected]>
|
@th33xitus yes sure, as soon as it helps landing this feature into master, i'm totally for it! |
Signed-off-by: Dominik Willner <[email protected]>
Signed-off-by: Dominik Willner <[email protected]>
Signed-off-by: Dominik Willner <[email protected]>
Huge PR with tons of changes. Mostly around implementing installing/removing instances on the fly without full reinstall. WIth some other bugs fixed on the go. Probably some new bugs introduces, requires good review.
For now I'd continue to work on streamer installation refactor with same technique used.EDIT: @KwadFan will implement crownest installation