diff --git a/jupyter_client/connect.py b/jupyter_client/connect.py index 70c94258b..cefc0b18b 100644 --- a/jupyter_client/connect.py +++ b/jupyter_client/connect.py @@ -573,7 +573,8 @@ def _reconcile_connection_info(self, info: KernelConnectionInfo) -> None: Because some provisioners (like derivations of LocalProvisioner) may have already written the connection file, this method needs to ensure that, if the connection file exists, its contents match that of what was returned by the provisioner. If - the file does exist and its contents do not match, a ValueError is raised. + the file does exist and its contents do not match, the file will be replaced with + the provisioner information (which is considered the truth). If the file does not exist, the connection information in 'info' is loaded into the KernelManager and written to the file. @@ -586,24 +587,24 @@ def _reconcile_connection_info(self, info: KernelConnectionInfo) -> None: if os.path.exists(self.connection_file): with open(self.connection_file) as f: file_info = json.load(f) - # Prior to the following comparison, we need to adjust the value of "key" to - # be bytes, otherwise the comparison below will fail. - file_info["key"] = file_info["key"].encode() - if not self._equal_connections(info, file_info): - raise ValueError( - "Connection file already exists and does not match " - "the expected values returned from provisioner!" - ) + # Prior to the following comparison, we need to adjust the value of "key" to + # be bytes, otherwise the comparison below will fail. + file_info["key"] = file_info["key"].encode() + if not self._equal_connections(info, file_info): + os.remove(self.connection_file) # Contents mismatch - remove the file + self._connection_file_written = False + else: file_exists = True if not file_exists: - # Load the connection info and write out file. Note, this does not necessarily - # overwrite non-zero port values, so we'll validate afterward. + # Load the connection info and write out file, clearing existing + # port-based attributes so they will be reloaded + for name in port_names: + setattr(self, name, 0) self.load_connection_info(info) self.write_connection_file() - # Ensure what is in KernelManager is what we expect. This will catch issues if the file - # already existed, yet it's contents differed from the KernelManager's (and provisioner). + # Ensure what is in KernelManager is what we expect. km_info = self.get_connection_info() if not self._equal_connections(info, km_info): raise ValueError( diff --git a/jupyter_client/tests/test_connect.py b/jupyter_client/tests/test_connect.py index f68082f7d..1289d2638 100644 --- a/jupyter_client/tests/test_connect.py +++ b/jupyter_client/tests/test_connect.py @@ -240,15 +240,15 @@ def test_mixin_cleanup_random_ports(): param_values = [ - (True, True, None), - (True, False, ValueError), - (False, True, None), - (False, False, ValueError), + (True, True), + (True, False), + (False, True), + (False, False), ] -@pytest.mark.parametrize("file_exists, km_matches, expected_exception", param_values) -def test_reconcile_connection_info(file_exists, km_matches, expected_exception): +@pytest.mark.parametrize("file_exists, km_matches", param_values) +def test_reconcile_connection_info(file_exists, km_matches): expected_info = sample_info mismatched_info = sample_info.copy() @@ -272,9 +272,10 @@ def test_reconcile_connection_info(file_exists, km_matches, expected_exception): provisioner_info = info km.load_connection_info(provisioner_info) else: - # Let this be the case where the connection file exists, the KM has no values - # prior to reconciliation, but the provisioner has returned different values - # and a ValueError is expected. + # Let this be the case where the connection file exists, and the KM has those values + # that differ from the ones returned by the provisioner. This is the restart-with- + # changed-ports case (typical for remote provisioners). + km.load_connection_info(expected_info) provisioner_info = mismatched_info else: # connection file does not exist if km_matches: @@ -284,20 +285,11 @@ def test_reconcile_connection_info(file_exists, km_matches, expected_exception): provisioner_info = expected_info else: # Let this be the case where the connection file does not exist, yet the KM - # has values that do not match those returned from the provisioner and a - # ValueError is expected. + # has values that do not match those returned from the provisioner. This case + # is probably not practical and is equivalent to the True, False case. km.load_connection_info(expected_info) provisioner_info = mismatched_info - if expected_exception is None: - km._reconcile_connection_info(provisioner_info) - km_info = km.get_connection_info() - assert km._equal_connections(km_info, provisioner_info) - else: - with pytest.raises(expected_exception) as ex: - km._reconcile_connection_info(provisioner_info) - if file_exists: - assert "Connection file already exists" in str(ex.value) - else: - assert "KernelManager's connection information already exists" in str(ex.value) - assert km._equal_connections(km.get_connection_info(), provisioner_info) is False + km._reconcile_connection_info(provisioner_info) + km_info = km.get_connection_info() + assert km._equal_connections(km_info, provisioner_info)