From 5c6b680259192c94c76298a81f3ca7f83ccc8185 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 12 Jun 2023 22:54:46 +0100 Subject: [PATCH 1/3] check proxy remote uri if is byref OR is not formatted value simplify condition proxykernel should not remove targetkernel name revert proxy kernel changes check is proxy remoteuri clean test --- .../VariableSharingTests.SetMagicCommand.cs | 37 ++++++++++++ .../Connection/ProxyKernel.cs | 2 +- .../KernelExtensions.cs | 60 +++++++------------ 3 files changed, 58 insertions(+), 41 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs b/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs index 9c57c87134..92a06210b1 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs @@ -494,6 +494,43 @@ public async Task value_option_is_required() .Which.Message.Should().Be("Option '--value' is required."); } + [Fact] + public async Task ProxyKernels_can_share_values() + { + using var localCompositeKernel = new CompositeKernel + { + (new CSharpKernel()).UseValueSharing() + }; + using var remoteCompositeKernel = new CompositeKernel + { + (new FSharpKernel()).UseValueSharing() + }; + + ConnectHost.ConnectInProcessHost( + localCompositeKernel, + remoteCompositeKernel); + + await localCompositeKernel + .Host + .ConnectProxyKernelOnDefaultConnectorAsync( + "fsharp", + new Uri("kernel://remote/fsharp")); + + await remoteCompositeKernel.SendAsync(new SubmitCode("let x = 123")); + + var result = await localCompositeKernel.SendAsync(new SubmitCode(""" + #!set --name x --value @fsharp:x + x + """, targetKernelName: "csharp")); + + result.Events.Should() + .ContainSingle() + .Which + .Value + .Should() + .Be(123); + } + private static Kernel CreateKernel(Language language) { return language switch diff --git a/src/Microsoft.DotNet.Interactive/Connection/ProxyKernel.cs b/src/Microsoft.DotNet.Interactive/Connection/ProxyKernel.cs index 7ec9a779be..f0972006da 100644 --- a/src/Microsoft.DotNet.Interactive/Connection/ProxyKernel.cs +++ b/src/Microsoft.DotNet.Interactive/Connection/ProxyKernel.cs @@ -98,7 +98,6 @@ private Task HandleByForwardingToRemoteAsync(KernelCommand command, KernelInvoca return Task.CompletedTask; } } - var targetKernelName = command.TargetKernelName; if (command.TargetKernelName == Name) { @@ -128,6 +127,7 @@ private Task HandleByForwardingToRemoteAsync(KernelCommand command, KernelInvoca { command.TargetKernelName = targetKernelName; + if (te.Result is CommandFailed cf) { context.Fail(command, cf.Exception, cf.Message); diff --git a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs index 56b776d811..596aaefbf7 100644 --- a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs @@ -173,51 +173,29 @@ private static async Task HandleSetMagicCommand( var valueOptionResult = cmdLineContext.ParseResult.GetValueForOption(valueOption); - // FIX: (HandleSetMagicCommand) - - switch (events.Count) - { - case 0: break; - case 1: break; - case 2: break; - default: break; - } - - if (valueOptionResult is { }) - { - if (valueOptionResult.Kernel is { } sourceKernelName) - { - switch (sourceKernelName) - { - case "input": break; - case "password": break; - default: break; - } - } - else - { - } - - if (valueOptionResult.Kernel is { } sourceValueName) - { - } - else - { - } + var sourceKernel = Kernel.Root.FindKernelByName(valueOptionResult.Kernel); + + ValueProduced valueProduced; + if ( + sourceKernel?.KernelInfo.IsProxy == false + && valueOptionResult is { Name: var sourceValueName, Kernel: var sourceKernelName } + && sourceKernelName != "input") + { + valueProduced = events.SingleOrDefault(e => + e.Name == sourceValueName && e.Command.TargetKernelName == sourceKernelName); + }else if (sourceKernel?.KernelInfo.IsProxy == true + && valueOptionResult is { Name: var sourceValueName1 }) + { + var destinationUri = sourceKernel?.KernelInfo.RemoteUri; + + valueProduced = events.SingleOrDefault(e => + e.Name == sourceValueName1 && e.Command.DestinationUri == destinationUri); } else { + valueProduced = null; } - var valueProduced = valueOptionResult switch - { - { Name: var sourceValueName, Kernel: var sourceKernelName } when - // !string.IsNullOrWhiteSpace(sourceKernelName) && - sourceKernelName != "input" => events.SingleOrDefault( - e => e.Name == sourceValueName && e.Command.TargetKernelName == sourceKernelName), - _ => null - }; - if (valueProduced is { }) { var referenceValue = isByref ? valueProduced.Value : null; @@ -383,6 +361,8 @@ ValueOptionResult ParseValueOption(ArgumentResult argResult) isByref = false; } + var valueSourceKernel = destinationKernel.RootKernel.FindKernelByName(sourceValueName); + var result = destinationKernel.RootKernel.SendAsync(requestValue).GetAwaiter().GetResult(); if (result.Events.LastOrDefault() is CommandFailed failed) From f799ac6b2135cb90f4e9483409f28b8a775c0b3b Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 12 Jun 2023 22:57:01 +0100 Subject: [PATCH 2/3] rename type --- .../VariableSharingTests.SetMagicCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs b/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs index 92a06210b1..3fc65729ff 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs @@ -495,7 +495,7 @@ public async Task value_option_is_required() } [Fact] - public async Task ProxyKernels_can_share_values() + public async Task ProxyKernels_sharing_values_receives_them_as_deserialized_values() { using var localCompositeKernel = new CompositeKernel { From e5b828b365c4b0bef9e4c78e9d194fb7fe704068 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 12 Jun 2023 22:58:36 +0100 Subject: [PATCH 3/3] clean code --- src/Microsoft.DotNet.Interactive/KernelExtensions.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs index 596aaefbf7..5a4dd958de 100644 --- a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs @@ -360,8 +360,6 @@ ValueOptionResult ParseValueOption(ArgumentResult argResult) requestValue = new RequestValue(sourceValueName, JsonFormatter.MimeType, sourceKernelName); isByref = false; } - - var valueSourceKernel = destinationKernel.RootKernel.FindKernelByName(sourceValueName); var result = destinationKernel.RootKernel.SendAsync(requestValue).GetAwaiter().GetResult();