Skip to content

Commit 205a8d2

Browse files
aleks-ivanovaleks-ivanov
committed
Merge branch 'main' into feature/codeql
2 parents d6b49df + da638ac commit 205a8d2

File tree

11 files changed

+133
-57
lines changed

11 files changed

+133
-57
lines changed

‎src/csharp/Extensions/Microsoft.Spark.Extensions.DotNet.Interactive.UnitTest/Microsoft.Spark.Extensions.DotNet.Interactive.UnitTest.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
</PropertyGroup>
77

88
<ItemGroup>
9+
<PackageReference Include="Microsoft.DotNet.Interactive.CSharp" Version="1.0.0-beta.25323.1" />
910
<PackageReference Include="Moq" Version="4.20.70" />
10-
<PackageReference Include="Microsoft.DotNet.Interactive" Version="1.0.0-beta.20480.3" />
1111
</ItemGroup>
1212

1313
<ItemGroup>

‎src/csharp/Extensions/Microsoft.Spark.Extensions.DotNet.Interactive.UnitTest/PackageResolverTests.cs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.Collections.Generic;
67
using System.IO;
78
using System.Linq;
@@ -40,7 +41,7 @@ public void TestPackageResolver()
4041
};
4142
var probingPaths = new string[] { packageRootPath };
4243

43-
var mockSupportNugetWrapper = new Mock<SupportNugetWrapper>();
44+
var mockSupportNugetWrapper = new Mock<ReferencedPackagesExtractor>(null);
4445
mockSupportNugetWrapper
4546
.SetupGet(m => m.ResolvedPackageReferences)
4647
.Returns(new ResolvedPackageReference[]
@@ -54,20 +55,19 @@ public void TestPackageResolver()
5455
});
5556

5657
var packageResolver = new PackageResolver(mockSupportNugetWrapper.Object);
57-
IEnumerable<string> actualFiles = packageResolver.GetFiles(tempDir.Path);
58+
string[] actualFiles = packageResolver.GetFiles(tempDir.Path).ToArray();
59+
string actualNugetPath = actualFiles[0];
60+
string actualMetadataPath = actualFiles[1];
61+
string actualMetadataFilename = Path.GetFileName(actualMetadataPath);
5862

59-
string metadataFilePath =
60-
Path.Combine(tempDir.Path, DependencyProviderUtils.CreateFileName(1));
61-
var expectedFiles = new string[]
62-
{
63-
nugetFile.FullName,
64-
metadataFilePath
65-
};
66-
Assert.True(expectedFiles.SequenceEqual(actualFiles));
67-
Assert.True(File.Exists(metadataFilePath));
63+
Assert.Equal(2, actualFiles.Length);
64+
Assert.Equal(nugetFile.FullName, actualNugetPath);
65+
Assert.StartsWith(tempDir.Path, actualMetadataPath);
66+
Assert.Matches("dependencyProviderMetadata_[a-f\\d]{8}00000000001", actualMetadataFilename);
67+
Assert.True(File.Exists(actualMetadataPath));
6868

6969
DependencyProviderUtils.Metadata actualMetadata =
70-
DependencyProviderUtils.Metadata.Deserialize(metadataFilePath);
70+
DependencyProviderUtils.Metadata.Deserialize(actualMetadataPath);
7171
var expectedMetadata = new DependencyProviderUtils.Metadata
7272
{
7373
AssemblyProbingPaths = new string[]
@@ -89,7 +89,7 @@ public void TestPackageResolver()
8989
}
9090
}
9191
};
92-
Assert.True(expectedMetadata.Equals(actualMetadata));
92+
Assert.Equal(expectedMetadata, actualMetadata);
9393
}
9494
}
9595
}

‎src/csharp/Extensions/Microsoft.Spark.Extensions.DotNet.Interactive/AssemblyKernelExtension.cs‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
using Microsoft.DotNet.Interactive;
1111
using Microsoft.DotNet.Interactive.Commands;
1212
using Microsoft.DotNet.Interactive.CSharp;
13-
using Microsoft.DotNet.Interactive.Utility;
1413
using Microsoft.Spark.Interop;
1514
using Microsoft.Spark.Sql;
1615
using Microsoft.Spark.Utils;
@@ -26,9 +25,10 @@ public class AssemblyKernelExtension : IKernelExtension
2625
{
2726
private const string TempDirEnvVar = "DOTNET_SPARK_EXTENSION_INTERACTIVE_TMPDIR";
2827
private const string PreserveTempDirEnvVar = "DOTNET_SPARK_EXTENSION_INTERACTIVE_PRESERVE_TMPDIR";
28+
private const string CsharpKernelName = "csharp";
2929

30-
private readonly PackageResolver _packageResolver =
31-
new PackageResolver(new SupportNugetWrapper());
30+
private ReferencedPackagesExtractor _referencedPackagesContainer;
31+
private PackageResolver _packageResolver;
3232

3333
/// <summary>
3434
/// Called by the Microsoft.DotNet.Interactive Assembly Extension Loader.
@@ -39,14 +39,23 @@ public Task OnLoadAsync(Kernel kernel)
3939
{
4040
if (kernel is CompositeKernel compositeKernel)
4141
{
42+
var cSharpKernel = kernel.FindKernelByName(CsharpKernelName) as CSharpKernel;
43+
44+
_referencedPackagesContainer = new ReferencedPackagesExtractor(cSharpKernel);
45+
_packageResolver = new PackageResolver(_referencedPackagesContainer);
46+
4247
Environment.SetEnvironmentVariable(Constants.RunningREPLEnvVar, "true");
4348

44-
DirectoryInfo tempDir = CreateTempDirectory();
49+
var tempDir = new DirectoryInfo(Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()));
50+
tempDir.Create();
4551

46-
if (!EnvironmentUtils.GetEnvironmentVariableAsBool(PreserveTempDirEnvVar))
52+
kernel.RegisterForDisposal(() =>
4753
{
48-
compositeKernel.RegisterForDisposal(new DisposableDirectory(tempDir));
49-
}
54+
if (!EnvironmentUtils.GetEnvironmentVariableAsBool(PreserveTempDirEnvVar))
55+
{
56+
tempDir.Delete(true);
57+
}
58+
});
5059

5160
compositeKernel.AddMiddleware(async (command, context, next) =>
5261
{
@@ -106,10 +115,25 @@ private bool TryEmitAssembly(CSharpKernel kernel, string dstPath, out string ass
106115
$"TryEmitAssembly() unexpected duplicate assembly: ${assemblyPath}");
107116
}
108117

118+
/// <summary>
119+
/// Uses <see cref="SparkSession.GetActiveSession"/> to retrieve active or default session if one exists
120+
/// Otherwise returns <see cref="SparkSession.GetDefaultSession"/>
121+
/// </summary>
122+
/// <param name="sparkSession">Out variable, that is set to active Spark Session if one found</param>
123+
/// <returns>True if spark session is found, otherwise False.</returns>
109124
private bool TryGetSparkSession(out SparkSession sparkSession)
110125
{
111-
sparkSession = SparkSession.GetActiveSession();
112-
return sparkSession != null;
126+
sparkSession = default;
127+
128+
try
129+
{
130+
sparkSession = SparkSession.GetActiveSession() ?? SparkSession.GetDefaultSession();
131+
return sparkSession != null;
132+
}
133+
catch (Exception ex) when (ex.InnerException is JvmException)
134+
{
135+
return false;
136+
}
113137
}
114138

115139
private IEnumerable<string> GetPackageFiles(string path)

‎src/csharp/Extensions/Microsoft.Spark.Extensions.DotNet.Interactive/Microsoft.Spark.Extensions.DotNet.Interactive.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
</ItemGroup>
2121

2222
<ItemGroup>
23-
<PackageReference Include="Microsoft.DotNet.Interactive.CSharp" Version="1.0.0-beta.20480.3">
23+
<PackageReference Include="Microsoft.DotNet.Interactive.CSharp" Version="1.0.0-beta.25323.1">
2424
<PrivateAssets>all</PrivateAssets>
2525
</PackageReference>
2626
</ItemGroup>

‎src/csharp/Extensions/Microsoft.Spark.Extensions.DotNet.Interactive/PackageResolver.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.Collections.Concurrent;
67
using System.Collections.Generic;
78
using System.IO;
@@ -13,13 +14,14 @@ namespace Microsoft.Spark.Extensions.DotNet.Interactive
1314
{
1415
internal class PackageResolver
1516
{
16-
private readonly SupportNugetWrapper _supportNugetWrapper;
17+
private readonly ReferencedPackagesExtractor _referencedPackagesExtractor;
1718
private readonly ConcurrentDictionary<string, byte> _filesCopied;
1819
private long _metadataCounter;
20+
private Guid _runId = Guid.NewGuid();
1921

20-
internal PackageResolver(SupportNugetWrapper supportNugetWrapper)
22+
internal PackageResolver(ReferencedPackagesExtractor referencedPackagesExtractor)
2123
{
22-
_supportNugetWrapper = supportNugetWrapper;
24+
_referencedPackagesExtractor = referencedPackagesExtractor;
2325
_filesCopied = new ConcurrentDictionary<string, byte>();
2426
_metadataCounter = 0;
2527
}
@@ -92,6 +94,7 @@ internal IEnumerable<string> GetFiles(string writePath)
9294
Path.Combine(
9395
writePath,
9496
DependencyProviderUtils.CreateFileName(
97+
_runId,
9598
Interlocked.Increment(ref _metadataCounter)));
9699
new DependencyProviderUtils.Metadata
97100
{
@@ -111,9 +114,7 @@ internal IEnumerable<string> GetFiles(string writePath)
111114
/// <returns>The delta of the list of packages.</returns>
112115
private IEnumerable<ResolvedNuGetPackage> GetNewPackages()
113116
{
114-
IEnumerable<ResolvedPackageReference> packages =
115-
_supportNugetWrapper.ResolvedPackageReferences;
116-
foreach (ResolvedPackageReference package in packages)
117+
foreach (var package in _referencedPackagesExtractor.ResolvedPackageReferences)
117118
{
118119
var packageRootDirectory = new DirectoryInfo(package.PackageRoot);
119120
IEnumerable<FileInfo> files =

‎src/csharp/Extensions/Microsoft.Spark.Extensions.DotNet.Interactive/ReferencedPackagesExtractor.cs‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Reflection;
5+
using Microsoft.DotNet.Interactive;
6+
using Microsoft.DotNet.Interactive.CSharp;
7+
8+
namespace Microsoft.Spark.Extensions.DotNet.Interactive
9+
{
10+
internal class ReferencedPackagesExtractor
11+
{
12+
private readonly CSharpKernel _kernel;
13+
14+
public ReferencedPackagesExtractor(CSharpKernel kernel)
15+
{
16+
_kernel = kernel;
17+
}
18+
19+
/// <summary>
20+
/// Gets collection of <see cref="ResolvedPackageReference"/>, extracted from <see cref="CSharpKernel"/> using reflection.
21+
/// </summary>
22+
/// <remarks>
23+
/// This implementation uses a fragile approach, relying on reflection to access
24+
/// the private "_disposables" field and internal "PackageRestoreContext" type within
25+
/// the kernel. This is necessary because a public API for retrieving this
26+
/// information is not available. This code is susceptible to breaking in future
27+
/// versions of the Microsoft.DotNet.Interactive library if its internal
28+
/// structure changes.
29+
/// https://github.com/dotnet/spark/discussions/1179
30+
/// </remarks>
31+
internal virtual IEnumerable<ResolvedPackageReference> ResolvedPackageReferences
32+
{
33+
get
34+
{
35+
var disposablesField = typeof(Microsoft.DotNet.Interactive.Kernel).GetField("_disposables", BindingFlags.Instance | BindingFlags.NonPublic);
36+
var disposables = disposablesField?.GetValue(_kernel) as IEnumerable<IDisposable>;
37+
38+
if (disposables is null)
39+
{
40+
throw new Exception("Failed to retrieve referenced packages from kernel, try using older version of Dotnet.Interactive");
41+
}
42+
43+
// Find the PackageRestoreContext instance in internal collection of disposables
44+
var restoreContext = disposables
45+
.FirstOrDefault(d => d.GetType().FullName == "Microsoft.DotNet.Interactive.PackageManagement.PackageRestoreContext");
46+
47+
if (restoreContext is null)
48+
{
49+
throw new Exception("PackageRestoreContext was not found in the kernel, try using older version of Dotnet.Interactive");
50+
}
51+
52+
// Extract the ResolvedPackageReferences property (internal)
53+
var resolvedPackagesProp = restoreContext
54+
.GetType()
55+
.GetProperty("ResolvedPackageReferences", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
56+
57+
return resolvedPackagesProp?.GetValue(restoreContext) as IEnumerable<ResolvedPackageReference>;
58+
}
59+
}
60+
}
61+
}

‎src/csharp/Extensions/Microsoft.Spark.Extensions.DotNet.Interactive/SupportNugetWrapper.cs‎

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎src/csharp/Microsoft.Spark.UnitTest/DependencyProviderUtilsTests.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.IO;
23
using System.Linq;
34
using Microsoft.Spark.UnitTest.TestUtils;
@@ -130,19 +131,20 @@ public void TestMetadataSerDe()
130131
[Fact]
131132
public void TestFileNames()
132133
{
134+
Guid runId = Guid.NewGuid();
133135
using var tempDir = new TemporaryDirectory();
134136
foreach (long num in Enumerable.Range(0, 3).Select(x => System.Math.Pow(10, x)))
135137
{
136138
string filePath =
137-
Path.Combine(tempDir.Path, DependencyProviderUtils.CreateFileName(num));
139+
Path.Combine(tempDir.Path, DependencyProviderUtils.CreateFileName(runId, num));
138140
File.Create(filePath).Dispose();
139141
}
140142

141143
var expectedFiles = new string[]
142144
{
143-
"dependencyProviderMetadata_0000000000000000001",
144-
"dependencyProviderMetadata_0000000000000000010",
145-
"dependencyProviderMetadata_0000000000000000100",
145+
$"dependencyProviderMetadata_{runId.ToString("N").Substring(0, 8)}00000000001",
146+
$"dependencyProviderMetadata_{runId.ToString("N").Substring(0, 8)}00000000010",
147+
$"dependencyProviderMetadata_{runId.ToString("N").Substring(0, 8)}00000000100",
146148
};
147149
IOrderedEnumerable<string> actualFiles = DependencyProviderUtils
148150
.GetMetadataFiles(tempDir.Path)

‎src/csharp/Microsoft.Spark.Worker.UnitTest/DependencyProviderTests.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.IO;
67
using System.IO.Compression;
78
using Microsoft.Spark.UnitTest.TestUtils;
@@ -47,12 +48,12 @@ public void TestsUnpackPackages()
4748

4849
using var unpackDir = new TemporaryDirectory();
4950
string metadataFilePath =
50-
Path.Combine(nupkgDir.Path, DependencyProviderUtils.CreateFileName(1));
51+
Path.Combine(nupkgDir.Path, DependencyProviderUtils.CreateFileName(Guid.NewGuid(), 1));
5152
metadata.Serialize(metadataFilePath);
5253

5354
// Files located in nupkgDir
5455
// nuget: package.name.1.0.0.nupkg
55-
// metadata file: dependencyProviderMetadata_00000000000000000001
56+
// metadata file: dependencyProviderMetadata_f1a2b3c400000000001
5657
var dependencyProvider =
5758
new DependencyProvider(metadataFilePath, nupkgDir.Path, unpackDir.Path);
5859
string expectedPackagePath =

‎src/csharp/Microsoft.Spark.Worker/Utils/AssemblyLoaderHelper.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ static AssemblyLoaderHelper()
4949
/// These files include:
5050
/// - "{packagename}.{version}.nupkg"
5151
/// The nuget packages
52-
/// - <see cref="DependencyProviderUtils.CreateFileName(long)"/>
52+
/// - <see cref="DependencyProviderUtils.CreateFileName(Guid, long)"/>
5353
/// Serialized <see cref="DependencyProviderUtils.Metadata"/> object.
5454
///
5555
/// On the Worker, in order to resolve the nuget dependencies referenced by

0 commit comments

Comments
 (0)