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

Reset spyder and restart from within running application #2423

Merged
merged 25 commits into from
Jul 17, 2015
Merged
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
17395a3
Reset spyder and restart from running application
goanpeca May 7, 2015
01c2188
Fix response for subprocess
goanpeca Jun 11, 2015
037e59d
Add option to preferences dialog
goanpeca Jun 11, 2015
58d3772
Rebased
goanpeca Jun 11, 2015
6c9f61f
Add traceback print error handling and pep8 corrections
goanpeca Jun 11, 2015
e04aca3
Add restart, reset and close extra error handling
goanpeca Jun 11, 2015
519201a
Improve code style
goanpeca Jun 12, 2015
bc95a72
Change timer settings
goanpeca Jun 12, 2015
13330fc
Change timer settings
goanpeca Jun 12, 2015
f0ec3f5
Update reset logic
goanpeca Jun 12, 2015
4003c37
Fix duplicate range
goanpeca Jun 12, 2015
28fc618
Rework exception handling
goanpeca Jun 13, 2015
eb8b585
Update error codes
goanpeca Jun 18, 2015
d341b5c
Update error codes
goanpeca Jun 18, 2015
4e9b789
Improve comments and variable names
goanpeca Jul 1, 2015
a6f8646
Reorganize code in config dialog
goanpeca Jul 1, 2015
a0d7a5f
Add splash screen
goanpeca Jul 2, 2015
3f4b398
Using current sys.path to restart app
SylvainCorlay Jul 8, 2015
a738d31
Using current sys.path to restart app
SylvainCorlay Jul 8, 2015
4931b49
Fix check logic on restarter script
goanpeca Jul 8, 2015
363853c
Fix typo in docstring
goanpeca Jul 8, 2015
63ca7be
Modify path in restart method only in bootstrap mode
goanpeca Jul 11, 2015
73eab81
reduce waiting times on all OSs
goanpeca Jul 11, 2015
37e0535
Reduce max waiting time when resetting
goanpeca Jul 15, 2015
abdb8f9
Remove helper function adn use to_text_string from spyderlib.py3compat
goanpeca Jul 17, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Improve code style
  • Loading branch information
goanpeca committed Jul 11, 2015
commit 519201ac96b3c4895c81b3a750f10bc2ab0163b6
62 changes: 27 additions & 35 deletions spyderlib/restart_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@


PY2 = sys.version[0] == '2'
CLOSE_ERROR, RESET_ERROR, RESTART_ERROR = list(range(3))


def _is_pid_running_on_windows(pid):
Expand Down Expand Up @@ -63,28 +64,27 @@ def is_pid_running(pid):
return _is_pid_running_on_unix(pid)


def error_message(type_, error=None):
def launch_error_message(type_, error=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type_ is an ugly error variable name to me. Perhaps error_type would be better?

"""Launch a message box with a predefined error message.

Parameters
----------
type : str ['close', 'reset', 'restart']
type : int [CLOSE_ERROR, RESET_ERROR, RESTART_ERROR]
"""
import spyderlib.utils.icon_manager as ima
from spyderlib.baseconfig import _
from spyderlib.qt.QtGui import QMessageBox, QDialog
from spyderlib.utils import icon_manager as ima
from spyderlib.utils.qthelpers import qapplication

messages = {'close': _("The previous instance of Spyder has not closed."
"\nRestart aborted."),
'reset': _("Spyder could not reset to factory defaults.\n"
"Restart aborted."),
'restart': _("Spyder could not restart.\n"
"Restart aborted.")}
titles = {'close': _("Spyder exit error"),
'reset': _("Spyder reset error"),
'restart': _("Spyder restart error")}

messages = {CLOSE_ERROR: _("The previous instance of Spyder has not "
"closed.\nRestart aborted."),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a better message for this error would be:

It was not possible to close the previous Spyder instance.
Restart aborted.

RESET_ERROR: _("Spyder could not reset to factory defaults.\n"
"Restart aborted."),
RESTART_ERROR: _("Spyder could not restart.\n"
"Restart aborted.")}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another suggestion

It was not possible to restart Spyder.
Operation aborted.

titles = {CLOSE_ERROR: _("Spyder exit error"),
RESET_ERROR: _("Spyder reset error"),
RESTART_ERROR: _("Spyder restart error")}

if error:
e = error.__repr__()
Expand All @@ -101,8 +101,8 @@ def error_message(type_, error=None):
dlg = QDialog()
dlg.setVisible(False)
dlg.show()
QMessageBox.warning(None, title, message, QMessageBox.Ok)
raise Exception(message)
QMessageBox.warning(dlg, title, message, QMessageBox.Ok)
raise RuntimeError(message)


# Note: Copied from py3compat because we can't rely on Spyder
Expand Down Expand Up @@ -143,7 +143,7 @@ def main():
# Variables were stored as string literals in the environment, so to use
# them we need to parse them in a safe manner.
is_bootstrap = ast.literal_eval(is_bootstrap)
pid = int(pid)
pid = ast.literal_eval(pid)
args = ast.literal_eval(spyder_args)
reset = ast.literal_eval(reset)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What reset evals to? True or False? Just curious :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, I just saw below that this is the case ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-)


Expand Down Expand Up @@ -180,25 +180,22 @@ def main():
shell = os.name != 'nt'

# Wait for original process to end before launching the new instance
max_counter = 1500 # Max loops to perform before aborting
counter = 0
while counter < max_counter: # This is equivalent to ~ 5 minute (1500*0.2)
for counter in range(1500): # This is equivalent to ~ 5 minute (1500*0.2)
if not is_pid_running(pid):
break
time.sleep(0.2) # Throttling control
counter += 1

if counter == max_counter:
error_message(type_='close')
else:
# The old spyder instance took too long to close and restart aborts
launch_error_message(type_=CLOSE_ERROR)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the arg is called error_type, not type_

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops


env = os.environ.copy()

reset_ok = True

# Reset spyder if needed
# -------------------------------------------------------------------------
reset_ok = True

if reset:
# Build reset command
command_reset = '"{0}" "{1}" {2}'.format(python, spyder, args_reset)

try:
Expand All @@ -207,33 +204,28 @@ def main():
pid_reset = p.pid
except Exception as error:
p.kill()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p could be undefined if the error comes from Popen.
Error checking is complicated, even with exceptions !

error_message(type_='reset', error=error)
launch_error_message(type_=RESET_ERROR, error=error)

counter = 0
max_counter = 50
# Wait for reset process to end before restarting
while counter < max_counter: # This is ~= 10 seconds (50*0.2)
for counter in range(300): # This is ~= 1 minute (300*0.2)
if not is_pid_running(pid_reset):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there was an exception before, pid_reset won't be defined.
I think this block should go in an else: statement after the try: and except:, since you shouldn''t have to wait if there was an error (I guess).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I will make the adjustments, good suggestions

break
time.sleep(0.2) # Throttling control
counter += 1

if counter == max_counter:
else:
# The rest subprocess took too long and it is killed
p.kill()
reset_ok = False

# Restart
# -------------------------------------------------------------------------
if reset_ok:
# Try to restart
try:
p = subprocess.Popen(command, shell=shell, env=env)
except Exception as error:
p.kill()
error_message(type_='restart', error=error)
launch_error_message(type_=RESTART_ERROR, error=error)
else:
error_message(type_='reset')
launch_error_message(type_=RESET_ERROR)


if __name__ == '__main__':
Expand Down