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

Fix IPKernel config handling #315

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

kivel
Copy link

@kivel kivel commented Jan 27, 2025

Fix errors arising from disregarding the ipython-kernel-config.py

Description

Providing a kernel configuration makes the re-manager fail to load the environment with an ipython kernel.
See #314 for details.

Motivation and Context

When using an ipython kernel, users will expect the provided configuration for the kernel in the profile directory to be obeyed.
The bug resulted from explicitly defining a kernel-client with connection information produced in scope, rather than providing the IPKernelApp with the respective overwrites and creating the client from the KernelApp.

Summary of Changes for Release Notes

Fixed

No direct fixes as the expected behavior can be achieved by arranging the code.

Added

  • the find_kernel_ip function is made a little more robust in case the socket calls are failing

Changed

  • the sequence in which the IPKernelApp and the BlockingKernelClient are created are reversed.
  • the BlockingKernelClient is obtained via the provided method from IPKernelApp self._ip_kernel_client = self._ip_kernel_app.blocking_client()

Removed

  • self._ip_connect_file removed because is was never used.
  • self._ip_connect_info was removed because there is no real reason to keep this state, the info can be obtained from by calling self._ip_kernel_app.get_connection_info()
  • the calles to generate_random_port(kernel_ip) have been removed, as the IPKernelApp will used random ports be default, or read the ports from the config or an existing connection file.

How Has This Been Tested?

The configurations for two different profile setups are provided in issue #314.
Both cases work as expected.
Additionally, invoking start-re-manager --use-ipython-kernel=ON with a profile that doesn't bear a configuration, will behave identically to the existing implementation.

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

Successfully merging this pull request may close these issues.

1 participant