DYN-9702: Log when shared definitions folder is missing#17221
Closed
jasonstratton wants to merge 2 commits into
Closed
DYN-9702: Log when shared definitions folder is missing#17221jasonstratton wants to merge 2 commits into
jasonstratton wants to merge 2 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9702
Contributor
There was a problem hiding this comment.
Pull request overview
Adds low-level diagnostic logging to make a missing shared definitions folder observable at runtime, helping diagnose CI/build environments where the folder isn’t being deployed as expected.
Changes:
- Emit a
Trace.TraceWarningwhen the expected shared definitions directory (underCommonDataDirectory/definitions) does not exist. - Keep behavior unchanged: the directory is not created and the returned
DefinitionDirectorieslist is unchanged.
Comment on lines
223
to
+238
| if (Directory.Exists(commonDefinitionsDirectory) && | ||
| !definitionDirectories.Contains(commonDefinitionsDirectory, StringComparer.OrdinalIgnoreCase)) | ||
| { | ||
| definitionDirectories.Add(commonDefinitionsDirectory); | ||
| } | ||
| else if (!Directory.Exists(commonDefinitionsDirectory)) | ||
| { | ||
| // Diagnostic only: the shared "definitions" folder (e.g. Curve_Validate.dyf) | ||
| // is expected to be copied next to DynamoCore.dll at build time (see the | ||
| // Copy targets in DynamoCore.csproj). If it's missing, do not fabricate or | ||
| // add the path here -- just make the omission visible so this doesn't fail | ||
| // silently again (DYN-9702). PathManager has no injected logger, so this | ||
| // follows the existing low-level Trace-based diagnostic convention used | ||
| // elsewhere in DynamoCore instead of adding a new logging dependency. | ||
| Trace.TraceWarning("Expected shared definitions folder not found at: " + commonDefinitionsDirectory); | ||
| } |
Contributor
There was a problem hiding this comment.
I agree with this one is being evaluated twice
Add a Trace.TraceWarning in PathManager.DefinitionDirectories when the common "definitions" folder (containing Curve_Validate.dyf, copied by DynamoCore.csproj at build time) is not found at commonDataDir. This is diagnostic-only -- the returned directory list is unchanged, and we deliberately do not auto-create the folder (that approach was tried and reverted). Previously this case failed completely silently, which caused a hard-to-diagnose CI regression. PathManager has no injected logger, so this follows the existing Trace-based diagnostic convention used elsewhere in the codebase for logger-less, non-fatal issues rather than adding a new logging dependency.
jasonstratton
force-pushed
the
DYN-9702-Log-missing-shared-definitions-folder
branch
from
July 15, 2026 18:12
905f446 to
757615a
Compare
3 tasks
RobertGlobant20
approved these changes
Jul 15, 2026
RobertGlobant20
left a comment
RobertGlobant20
left a comment
Contributor
There was a problem hiding this comment.
LGTM with one minor comment
Contributor
Author
|
Closing as a duplicate — superseded by #17222, which lands the same fix (null-guarding the PackageLoaderTests predicates so the non-package Curve_Validate node with a null PackageInfo is skipped). Roberto's PR got there first. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
DYN-9702 introduced a shared "definitions" folder (shipped with a built-in custom node,
Curve_Validate.dyf) thatDynamoCore.csprojcopies next toDynamoCore.dllat build time, and thatPathManager.DefinitionDirectoriessurfaces to the rest of Dynamo.Regression introduced in: #17201, 2026-07-13.
If that folder is ever missing at runtime,
DefinitionDirectoriessilently omits it from its returned list with no error or log line -- this exact silent-drop is what causedPackageManagerTests.PackageLoaderTests.BuiltInPackagesIsNotExposedInPathManagerto fail deterministically on every daily CBCI build since 2026-07-14, with no visibility into why.This is a small, standalone fix: add a diagnostic warning when the folder is expected but not found. It intentionally does not change behavior (the folder is not created, the returned list is unchanged) -- it only makes the omission observable. We have not yet root-caused why the folder is missing specifically in the CBCI build environment (it is present in every local build); that investigation is ongoing and a follow-up commit/PR will land the actual fix once identified.
Key changes:
PathManager.DefinitionDirectoriesnow logs viaTrace.TraceWarningwhen the shared definitions folder doesn't exist at the expected path.PathManagerhas no injected logger, so this follows the existing low-levelTrace-based diagnostic precedent already used elsewhere inDynamoCore(e.g.AscSDKWrapper.cs) rather than adding a new logging dependency.Declarations
Release Notes
N/A -- diagnostic logging only, no user-facing or behavioral change.
Reviewers
(FILL ME IN) Reviewer 1
FYIs
(FILL ME IN, Optional)