Address loading of packages with inter-dependencies by aparajit-pratap · Pull Request #9736 · DynamoDS/Dynamo

aparajit-pratap · 2019-05-24T13:27:37Z

Purpose

This PR addresses the following issues (JIRA Link: https://jira.autodesk.com/browse/DYN-1863):

If there's a package A that depends on package B, and A does not come bundled with B, package B needs to be installed (from the PM) or loaded into Dynamo (imported in the VM) before A for A to be loaded successfully. If not, only B is loaded.

If B is already installed but not imported before A, resolve the dependencies from B that are required while importing A by searching for them in the package paths.
If package B is not already installed there is not much that can be done unless the user installs B manually. Once B is loaded/installed, we need to restart Dynamo in order to reload A.

If package A depends on B and also contains B's assembly, then A is imported successfully. In this case B will not be installed (nor imported). If B is installed from the PM separately, it will fail as the VM will throw a class redefinition error.

Still making sure that B is installed properly.
Now if B is labelled as "node library" in pkg.json manifest file it will load along with A.

Package load failures should now show up in notifications manager as shown:

Performance profiling results for loading 6 packages totaling 15,000 nodes during Dynamo startup (savings of ~10sec out of overall time of ~66sec):
Before:

After:

Declarations

Check these if you believe they are true

The code base is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@mjkkirschner

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

src/DynamoCore/Library/LibraryServices.cs

…tension

src/DynamoCore/Extensions/ExtensionLibraryLoader.cs

src/DynamoCore/Library/ILibraryLoader.cs

src/DynamoCore/Logging/DynamoLogger.cs

src/DynamoCore/Extensions/ReadyParams.cs

src/DynamoCore/Library/LibraryServices.cs

…nsion

… DYN files

src/DynamoCore/Library/LibraryServices.cs

src/Notifications/NotificationsViewExtension.cs

aparajit-pratap · 2019-06-13T23:15:44Z

test/Libraries/PackageManagerTests/PackageLoaderTests.cs

+            var entries = CurrentDynamoModel.SearchModel.SearchEntries.ToList();
+            Assert.IsTrue(entries.Any(x => x.FullName == "AnotherPackage.AnotherPackage.AnotherPackage.HelloAnotherWorld"));
+            Assert.IsTrue(entries.Any(x => x.FullName == "DependentPackage.DependentPackage.DependentPackage.HelloWorld"));
+            Assert.IsTrue(entries.Any(x => x.FullName == "Package.Package.Package.Hello"));


The following code shows the methods defined in the 3 test packages. DependentPackage has a dependency on Package and AnotherPackage has a dependency on the first two:

aparajit-pratap · 2019-06-14T00:35:11Z

test/DynamoCoreWpfTests/PublishPackageViewModelTests.cs

            string packagedirectory = Path.Combine(TestDirectory, "pkgs");
            var packages = Directory.EnumerateDirectories(packagedirectory);
-            var first = Path.GetFullPath(packages.First());
+            var first = Path.GetFullPath(packages.FirstOrDefault(x => Path.GetFileName(x).Equals("Custom Rounding")));


These tests specifically check for the "Custom Rounding" test package that earlier happened to be the first package in the list. This order changed in this PR after I added new test packages, which is why I'm specifically searching for the "Custom Rounding" package from now on.

test/DynamoCoreTests/DynamoModelTestBase.cs

mjkkirschner · 2019-06-14T15:13:09Z

LGTM!

* refactor package loading * change method name * refactor import packages during install * add import library step into VM after ImportLibrary is called * fixes in library services resolve assembly * try sending failure messages to notification view extension * surface package load exception to notifications view extension * clear notifications from logger after adding them to notifications extension * surface more library load failed exceptions to notification view extension * address review comments * revert unchanged file * obsolete public API's no longer used and to be removed in 3.0 * remove ReadyParams property * cleanup * correct for importing library from assembly info in old xml formatted DYN files * more cleanup * refactor LibraryServices methods * code cleanup, documentation * add reasons for obsoleting methods * add test for loading interdependent packages * cleanup * update failing tests from self-serve * revert unchanged file

aparajit-pratap added 4 commits May 23, 2019 11:24

refactor package loading

ee6c734

change method name

464b6c9

refactor import packages during install

217d94c

add import library step into VM after ImportLibrary is called

49fd77f

aparajit-pratap commented May 30, 2019

View reviewed changes

src/DynamoCore/Library/LibraryServices.cs Outdated Show resolved Hide resolved

fixes in library services resolve assembly

cbafa86

mjkkirschner reviewed May 31, 2019

View reviewed changes

src/DynamoCore/Library/LibraryServices.cs Show resolved Hide resolved

try sending failure messages to notification view extension

6e2b5c5

aparajit-pratap changed the title ~~[WIP] refactor package loading~~ [WIP] Address loading of packages with inter-dependencies Jun 4, 2019

aparajit-pratap added 3 commits June 5, 2019 13:35

resolve merge conflicts

d7b9e30

surface package load exception to notifications view extension

2f0c2e0

clear notifications from logger after adding them to notifications ex…

569391d

…tension