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-6672 - python engines failing to load should not stop Dynamo from starting #3040

Merged
merged 1 commit into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/DynamoRevit/DynamoRevit.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@
</Reference>
</ItemGroup>
<ItemGroup>
<Content Include="Ionic.Zip.xml" />
Copy link
Member Author

Choose a reason for hiding this comment

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

this file was deleted previously.

<EmbeddedResource Include="Resources\Category.Revit.svg" />
<EmbeddedResource Include="Resources\LayoutSpecs.json" />
<Content Include="Resources\logo_square_32x32.png" />
Expand Down
29 changes: 22 additions & 7 deletions src/DynamoRevit/Models/RevitDynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Windows.Forms;
using Autodesk.Revit.DB;
Expand Down Expand Up @@ -443,9 +444,16 @@ private void SetupPythonEngine(PythonEngine engine)
{
if (engine != null)
{
(engine.OutputDataMarshaler as DataMarshaler).RegisterMarshaler((Element element) => element.ToDSType(true));
engine.EvaluationStarted += OnPythonEvalStart;
engine.EvaluationFinished += OnPythonEvalFinished;
try
{
(engine.OutputDataMarshaler as DataMarshaler).RegisterMarshaler((Element element) => element.ToDSType(true));
engine.EvaluationStarted += OnPythonEvalStart;
engine.EvaluationFinished += OnPythonEvalFinished;
}
catch(FileNotFoundException ex)
{
Logger.Log(ex);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really wish there was a DynamoCore way to ensure that all PythonEngine public APIs are safe to use
If we go with this route, I think we might have to do the same thing for other host integrators

Copy link
Collaborator

@pinzart90 pinzart90 Feb 14, 2024

Choose a reason for hiding this comment

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

You cannot even mark an event as "Can throw Exception of type X". Oh we can...if that is any comfort

Copy link
Member Author

Choose a reason for hiding this comment

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

@pinzart90 I am looking into something on the DynamoCore side, I will send another PR for feedback on that. More a spike than something real at this point.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to propagate the error in DynamoCore and display it to the user somehow?

}
}

Expand All @@ -455,11 +463,18 @@ private void SetupPythonEngine(PythonEngine engine)
/// <param name="engine"></param>
private void CleanUpPythonEngine(PythonEngine engine)
{
if (engine != null)
try
{
if (engine != null)
{
(engine.OutputDataMarshaler as DataMarshaler).UnregisterMarshalerOfType<Element>();
engine.EvaluationStarted -= OnPythonEvalStart;
engine.EvaluationFinished -= OnPythonEvalFinished;
}
}
catch (FileNotFoundException ex)
{
(engine.OutputDataMarshaler as DataMarshaler).UnregisterMarshalerOfType<Element>();
engine.EvaluationStarted -= OnPythonEvalStart;
engine.EvaluationFinished -= OnPythonEvalFinished;
Logger.Log(ex);
}
}

Expand Down