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

DYN-7535 Record python engine package information in graphs when using engines served from them #15535

Merged
merged 12 commits into from
Oct 17, 2024
14 changes: 14 additions & 0 deletions src/DynamoCore/Graph/Nodes/NodeModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,20 @@ protected NodeModel(IEnumerable<PortModel> inPorts, IEnumerable<PortModel> outPo
RaisesModificationEvents = true;
}

/// <summary>
/// The method returns the assembly name from which the node originated.
/// </summary>
/// <returns>Assembly Name</returns>
internal virtual AssemblyName GetNameOfAssemblyReferencedByNode()
Copy link
Member

@mjkkirschner mjkkirschner Oct 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a better name is GetOwningAssemblyNameOfNodeModel - referenced by node sounds like a dependency of the node to me anyway.

{
AssemblyName assemblyName = null;

var assembly = this.GetType().Assembly;
assemblyName = AssemblyName.GetAssemblyName(assembly.Location);

return assemblyName;
}

/// <summary>
/// Here we try to find the correct port names and tooltips.
/// ideally we'd use the runtime information to correctly update or localize
Expand Down
20 changes: 19 additions & 1 deletion src/DynamoCore/Graph/Nodes/ZeroTouch/DSFunctionBase.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Xml;
using Dynamo.Engine;
using Dynamo.Engine.CodeGeneration;
Expand Down Expand Up @@ -76,6 +77,23 @@ public override IdentifierNode GetAstIdentifierForOutputIndex(int outputIndex)
}
}

/// <summary>
/// The method returns the assembly name from which the node originated.
/// </summary>
/// <returns>Assembly Name</returns>
internal override AssemblyName GetNameOfAssemblyReferencedByNode()
{
AssemblyName assemblyName = null;

var descriptor = this.Controller.Definition;
if (descriptor.IsPackageMember)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be valid even if this was not a packaged node -right?

Copy link
Contributor Author

@zeusongit zeusongit Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would be, but I think the intent is to return null for non-packaged nodes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then I think your summary/returns could be updated to reflect that / a test should be added encoding that behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just want to add that this was the default behavior, I did not add this code but moved it inside the base class. So whatever it was doing before will still be applied.

{
assemblyName = AssemblyName.GetAssemblyName(descriptor.Assembly);
}

return assemblyName;
}

/// <summary>
/// Copy command will call it to serialize this node to xml data.
/// </summary>
Expand Down Expand Up @@ -358,4 +376,4 @@ protected override AssociativeNode GetFunctionApplication(NodeModel model, List<
return rhs;
}
}
}
}
2 changes: 1 addition & 1 deletion src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,7 @@ internal PackageInfo GetNodePackage(NodeModel node)
throw new Exception("There are multiple subscribers to Workspace.CollectingNodePackageDependencies. " +
"Only PackageManagerExtension should subscribe to this event.");
}
var assemblyName = GetNameOfAssemblyReferencedByNode(node);
var assemblyName = node.GetNameOfAssemblyReferencedByNode();
if (assemblyName != null)
{
return CollectingNodePackageDependencies(assemblyName);
Expand Down
36 changes: 36 additions & 0 deletions src/DynamoPackages/PackageManagerExtension.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Linq;
using System.Reflection;
using Dynamo.Extensions;
Expand Down Expand Up @@ -145,6 +146,7 @@ public void Startup(StartupParams startupParams)
PackageLoader.PackagesLoaded += LoadPackagesHandler;
PackageLoader.RequestLoadNodeLibrary += RequestLoadNodeLibraryHandler;
PackageLoader.RequestLoadCustomNodeDirectory += RequestLoadCustomNodeDirectoryHandler;
PythonServices.PythonEngineManager.Instance.AvailableEngines.CollectionChanged += PythonEngineAdded;
zeusongit marked this conversation as resolved.
Show resolved Hide resolved

var dirBuilder = new PackageDirectoryBuilder(
new MutatingFileSystem(),
Expand All @@ -163,6 +165,40 @@ public void Startup(StartupParams startupParams)
noNetworkMode = startupParams.NoNetworkMode;
}

/// <summary>
/// When a new engine is added from a package, add its dependency to the respective package in the dictionary.
/// </summary>
/// <param name="sender"></param>
/// <param name="e"></param>
internal void PythonEngineAdded(object sender, NotifyCollectionChangedEventArgs e)
{
if (e.Action == NotifyCollectionChangedAction.Add)
{
var assem = e.NewItems[0].GetType().Assembly;
zeusongit marked this conversation as resolved.
Show resolved Hide resolved
var assemLoc = assem.Location;
foreach (var pkg in PackageLoader.LocalPackages)
{
if (assemLoc.StartsWith(pkg.RootDirectory))
{
if (NodePackageDictionary.ContainsKey(assem.FullName))
{
var assemName = AssemblyName.GetAssemblyName(assem.Location);
OnMessageLogged(LogMessage.Info(
string.Format("{0} contains the python engine library {1}, which has already been loaded " +
"by another package. This may cause inconsistent results when determining which " +
"python engine the nodes are dependent on.", pkg.Name, assemName.Name)
));
}
else
{
NodePackageDictionary[assem.FullName] = new List<PackageInfo>();
}
NodePackageDictionary[assem.FullName].Add(new PackageInfo(pkg.Name, new Version(pkg.VersionName)));
}
}
}
}

private PackageInfo handleCustomNodeOwnerQuery(Guid customNodeFunctionID)
{
return GetCustomNodePackageFromID(customNodeFunctionID);
Expand Down
20 changes: 19 additions & 1 deletion src/Libraries/PythonNodeModels/PythonNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.ComponentModel;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Xml;
using System.Xml.Serialization;
using Autodesk.DesignScript.Runtime;
Expand Down Expand Up @@ -59,8 +60,25 @@ public string EngineName
{
engine = value;
RaisePropertyChanged(nameof(EngineName));
}
}
}
}

/// <summary>
/// The method returns the assembly name from which the node originated.
/// </summary>
/// <returns>Assembly Name</returns>
internal override AssemblyName GetNameOfAssemblyReferencedByNode()
{
AssemblyName assemblyName = null;

var pyEng = PythonEngineManager.Instance.AvailableEngines.Where(x => x.Name.Equals(this.EngineName)).FirstOrDefault();
if (pyEng != null)
{
assemblyName = AssemblyName.GetAssemblyName(pyEng.GetType().Assembly.Location);
zeusongit marked this conversation as resolved.
Show resolved Hide resolved
}

return assemblyName;
}

/// <summary>
Expand Down
Loading