Allow pkgs to partially load if some of the assemblies are good #12763
Conversation
src/DynamoCore/Models/DynamoModel.cs
Outdated
| bool nodeModelOrNodeView; | ||
| try | ||
| { | ||
| nodeModelOrNodeView = !NodeModelAssemblyLoader.ContainsNodeModelSubType(assem) |
There was a problem hiding this comment.
These checks (ex ContainsNodeModelSubType) actually iterate over all the types in the assembly...so in the case of RhythmRevit.dll....it would throw a ReflectionTypeLoadException exception
| && !(NodeModelAssemblyLoader.ContainsNodeViewCustomizationType(assem)); | ||
| } | ||
| catch (Exception ex) { | ||
| throw new Exceptions.LibraryLoadFailedException(assem.Location, ex.Message); |
There was a problem hiding this comment.
any other type of Exception would disrupt the package loading operation.
Exceptions are managed in the same way inside LibraryServices.LoadNodeLibrary, i.e LibraryLoadFailedException usually means you can continue, anything else is fatal to the loading op
| if (loadedNodeLibs.Count > 0) | ||
| { | ||
| // Try to load any valid node libraries regardless package state | ||
| PackagesLoaded?.Invoke(loadedNodeLibs); |
There was a problem hiding this comment.
Why did you change the sequence of loading this by moving it up here from line 290?
There was a problem hiding this comment.
so that I do not have to add it in 2 places (when it succeeds and in the catch blocks)
There was a problem hiding this comment.
To avoid code duplication...I can put it here (line 283) or in a finally block (but I would have to tweak the loadedNodeLibs to be declared before the try catch)
src/DynamoPackages/PackageLoader.cs
Outdated
|
|
||
| if (blockedAssemblies.Count > 0) | ||
| { | ||
| throw new Exception("The following assemblies are blocked : " + string.Join(", ", blockedAssemblies.Select(x => Path.GetFileName(x.Location)))); |
There was a problem hiding this comment.
I think this should be new AssembyBlockedException for it to be caught here.
| // Generic fatal error | ||
| if (failedNodeLibs.Count > 0) | ||
| { | ||
| throw new Exception("Failed to load the following assemblies : " + string.Join(", ", failedNodeLibs.Select(x => Path.GetFileName(x.Location)))); |
There was a problem hiding this comment.
Shouldn't this be new LibraryLoadFailedException?
There was a problem hiding this comment.
well..it could be..but I don't think it matters...since we do not catch that in any special way in this try catch
|
Thank you all for your work on this. I am thrilled to see this as a "feature" rather than a "Random convenience" lol 😁 |
Purpose
Allow PackageLoader to partially load packages (only those assemblies that can be loaded should be loaded)
Original issue tracked here https://forum.dynamobim.com/t/cant-seem-to-install-packages-on-dynamo-sandbox/75000/12
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
(FILL ME IN) Brief description of the fix / enhancement. Mandatory section
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of