[Windows] Have a single instance of Agent Manager #1924
Conversation
elafarge
force-pushed
the
etienne/windows-singleton-manager
branch
from
e978acd to
c2f3031
Compare
| avoids multiple icons in the Tray on Windows. On OSX, we don't have to do anything: icons | ||
| don't get duplicated. """ | ||
| # On Windows, if a window is already opened, let's just bring it on the foreground | ||
| if Platform.is_windows(): |
There was a problem hiding this comment.
Nitpicking but you could check that before calling the function. That would avoid opening a new context for nothing if the platform is not Windows (and wrapping the whole thing in a big if).
There was a problem hiding this comment.
You are testing Platform.is_windows() two times.
|
Great success @elafarge! Except for a few nitpick I don't have much to say. Feel free to merge once you've considered the 2 comments. |
Unfortunately I can't squash this commit into the previous one since the branch has already been merged into the `etienne/windows-omnibus4` branch.
|
Thanks @hkaj :) I took your remarks into account. I'm right now building a Windows version of the Agent to run some tests. If it still works as expected, I'll merge that once and for all ;-) |
|
@yannmh I was wrong, since we're using the 5.5.x branch of the Agent, this is not gonna be shipped with the 5.5.0 by default. It can be an interesting set commits to cherrypick for the next bugfix release though ;-) |
[Windows] Have a single instance of Agent Manager
|
@elafarge don't forget to delete branches once merged |
| @@ -791,7 +799,52 @@ def info_popup(message, parent=None): | |||
| QMessageBox.information(parent, 'Message', message, QMessageBox.Ok) | |||
|
|
|||
|
|
|||
| def kill_old_process(): | |||
| """ Kills or brings to the foreground (if possible) any other instance of this program. It | |||
There was a problem hiding this comment.
or brings to the foreground (if possible) any other instance of this program
It looks like you actually always kill the previous instance in fact.
|
I know it has been merged, but I think killing the process rather than bringing the UI back is dangerous: for instance, any change made to a configuration file without saving would be lost. |
|
Oh yeah, actually it doesn't kill the process if the Window is opened, at least not on Windows server 2012: if an application is in the taskbar, starting it again from the launch menu will just bring the window to the front. I don't know if that behaviour is the same on Windows 7 likes (ie Windows server 2008). |
|
@yannmh ^^ |
When the GUI is started:
- It checks for a Pidfile left by a previously running (and potentially still running) GUI
- If such a Pidfile was found and if the Pid inside is file is indeed the Pid of a running instance of
agent-manager.exe, then let's kill that old process- Let's write the Pid of the current process to that pidfile
Nothing else was changed :)