PR: Get user environment variables asynchronously

[mrclary]

Under some circumstances, getting user environment variables may take some time. In order to not block Spyder's UI during the subprocess to get environment variables on unix systems, this is moved to a separate thread. This also allows a larger timeout to be specified.

• get_user_environment_variables is changed to a concurrent future object
• get_user_environment_variables is not called in the kernelspec module, but now in the ipythonconsole main widget module when creating a new client, and in the Pythonpath manager utils.
• Raise timeout for the subprocess to get user environment variables to 10s.
• Loading page appears in user environment variables dialog, pythonpath manager dialog, and IPython console, while retrieving environment variables.

[mrclary]

@hlouzada, thanks for helping me get started on implementing AsyncDispatcher. If you get a chance to take a look at this PR, I'd appreciate your feedback. I'm having some trouble with unit tests and IPython console behavior, but before I get into details, it would be nice to know if I've made some obvious high-level mistakes.

[hlouzada]

Overall everything looks good. But I noticed that there's no need for the get_user_environment_variables function to be run in it's own thread/event loop, since the get env is a shell function that starts a subprocesses, so you can just await for it to finish.

[mrclary]

@dalthviz, this seems to be working for all but one test on Windows conda.
The screencast shows the symptom: on startup, the prompt does not appear, although hitting return causes it to reappear. Nevertheless, it seems to be there for new consoles.

I can't figure out why this is happening; perhaps you have some ideas? I suspect this is some kind of race condition between the prompt and the banner.

[dalthviz]

@mrclary about the Windows test failing (test_clickable_ipython_tracebacks), checking the PR locally I was able to reproduce the test fail. As a way to make the test pass, seems like changing

spyder/spyder/app/tests/test_mainwindow.py

Lines 6707 to 6709 in 3606d9b

	# Run test file
	qtbot.mouseClick(main_window.run_button, Qt.LeftButton)
	qtbot.wait(500)

To something like

    # Run test file
    with qtbot.waitSignal(shell.sig_prompt_ready):
        qtbot.mouseClick(main_window.run_button, Qt.LeftButton)

helps

Checking the failing one on Linux (test_kernel_kill[True]), from the error output seems to me like the kernel is unable to restart after being killed 🤔 Since the test does a long wait before running the code used to kill the kernel to try to ensure the restart logic is setup, maybe somehow that long wait is interacting with the asynchronous nature of the changes this PR does? Maybe a way to make that test work could be replacing

spyder/spyder/plugins/ipythonconsole/tests/test_ipythonconsole.py

Lines 1395 to 1398 in 3606d9b

	shell = ipyconsole.get_current_shellwidget()
	# Wait for the restarter to start
	qtbot.wait(3000)

For something like

    shell = ipyconsole.get_current_shellwidget()
    control = shell._control
    qtbot.waitUntil(lambda: shell._prompt_html is not None,
                    timeout=SHELL_TIMEOUT)

?

[jitseniesen]

on startup, the prompt does not appear, although hitting return causes it to reappear. Nevertheless, it seems to be there for new consoles.

@mrclary I had a look at this and I think that I made some headway. Apologies for the long post.

I believe the relevant difference between the first console (the one created when Spyder starts up) and subsequent consoles is that in the first console, ShellWidget.set_color_scheme is called:

spyder/spyder/plugins/ipythonconsole/widgets/shell.py

Lines 725 to 739 in fc230d3

	def set_color_scheme(self, color_scheme, reset=True):
	"""Set color scheme of the shell."""
	self.set_bracket_matcher_color_scheme(color_scheme)
	self.style_sheet, dark_color = create_qss_style(color_scheme)
	self.syntax_style = color_scheme
	self._style_sheet_changed()
	self._syntax_style_changed(changed={})
	if reset:
	self.reset(clear=True)
	if not self.spyder_kernel_ready:
	# Will be sent later
	return
	self.set_kernel_configuration(
	"color scheme", "dark" if not dark_color else "light"
	)

I presume this function is called when the conf file with the colour scheme is read in and that this happens shortly after the first console is created but before user create any other consoles.

As the code shows, setting the colour scheme resets the console. Resetting the console prints a new prompt. Printing a prompt requires a kernel. If the kernel is not ready, then no prompt appears. With this PR, if getting the env vars takes some time, it may happen that set_color_scheme is called before the kernel is ready. One solution is to exchange the two if blocks in the code above. This has the effect of not resetting the console if the kernel is not ready. That solves the issue in my testing.

You may ask, why does no prompt appear if the console is reset before the kernel is ready? I think the answer lies in this function in qtconsole, which is called when the console is reset:

spyder/external-deps/qtconsole/qtconsole/jupyter_widget.py

Lines 365 to 389 in fc230d3

	def _show_interpreter_prompt(self, number=None):
	""" Reimplemented for IPython-style prompts.
	"""
	# If a number was not specified, make a prompt number request.
	if number is None:
	if self._prompt_requested:
	# Already asked for prompt, avoid multiple prompts.
	return
	self._prompt_requested = True
	msg_id = self.kernel_client.execute('', silent=True)
	info = self._ExecutionRequest(msg_id, 'prompt', False)
	self._request_info['execute'][msg_id] = info
	return
	# Show a new prompt and save information about it so that it can be
	# updated later if the prompt number turns out to be wrong.
	self._prompt_sep = self.input_sep
	self._show_prompt(self._make_in_prompt(number), html=True)
	block = self._control.document().lastBlock()
	length = len(self._prompt)
	self._previous_prompt_obj = self._PromptBlock(block, length, number)
	# Update continuation prompt to reflect (possibly) new prompt length.
	self._set_continuation_prompt(
	self._make_continuation_prompt(self._prompt), html=True)

This function is called with number = None and it "makes a prompt number request". If there is no kernel (which may happen when the kernel is slow to start), then self.kernel_client = None so the self.kernel_client.execute() fails (I guess an exception is raised which is silently somewhere swallowed). Even worse, self._prompt_requested is set to True so on subsequent calls (e.g., when the kernel has finally started up) no further requests will be made. A fix here is to change

            if self._prompt_requested:

to

            if self._prompt_requested or self.kernel_client is None:

This also fixes the issue in my testing.

[mrclary]

Thanks @dalthviz and @jitseniesen! I haven't had a chance to incorporate your suggestions yet, but I wanted you to know that they are much appreciated and I will be working on them soon.

[mrclary]

I've confirmed @jitseniesen recommendations worked! 🥇

[mrclary]

Looks like @dalthviz suggestions did the trick for the tests!

This should be ready for review now that everything is working.

Thanks for everyone's help!

[jitseniesen]

Unfortunately, this PR seems to break the spyder-notebook plugin. That plugin uses the same kernel spec as Spyder. With this PR, the notebook server spits out many errors, starting with:

Exception in callback BaseAsyncIOLoop._handle_events(9, 1)
handle: <Handle BaseAsyncIOLoop._handle_events(9, 1)>
Traceback (most recent call last):
  File "/local/data/amtjn/miniforge/envs/spydev/lib/python3.11/asyncio/events.py", line 84, in _run
    self._context.run(self._callback, *self._args)
RuntimeError: cannot enter context: <_contextvars.Context object at 0x7f7c804eb2c0> is already entered

I have very little experience with async programming in Python and I don't understand this error.

By trial and error, I found the breaking change: commit 8c74b90, specifically changing the decorator of get_user_environment_variables in spyder/utils/environ.py from @AsyncDispatcher(loop="get_user_env") to @AsyncDispatcher, which makes it use the default event loop. The function get_user_environment_variablesis actually never called in spyder-notebook, but it seems that the decorator does something when the file is imported which interferes with the notebook server.

Fortunately, this suggests an easy fix: Remove the import from kernelspec.py. After the PR, kernelspec.py uses only one function in environ.py and that function is effectively a one-liner so it can be easily inlined. I'll make a suggestion with the necessary changes (unfortunately, the GitHub suggestion feature does not work on lines that are not touched by the PR, so you'll have to apply my suggestions by hand).

@hlouzada Perhaps you know whether this makes sense or can suggest a better fix?

[mrclary]

@hlouzada Perhaps you know whether this makes sense or can suggest a better fix?

@hlouzada, you made the code suggestion earlier that removed loop="get_user_env" with the comment:

As per convention, all non-plugins coroutine should be run in the default event loop.

I'm wondering if this implies that all plugins should be using their own event loop. While I have no objection to @jitseniesen's recommendation, per se, if the issue arises because of a collision in the main event loop, then the issue @jitseniesen sees could affect any plugin that directly or indirectly imports from spyder.utils.environ.py. In the present case, should the notebook plugin create a dedicated event loop to handle all of its async operations?

[ccordoba12]

@mrclary, last comment for you then this should be ready.

[ccordoba12]

Looks good to me now, thanks @mrclary!