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-7409: Properly use the userData CLI parameter #15558

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

twastvedt
Copy link
Contributor

@twastvedt twastvedt commented Oct 17, 2024

Fixes the following bug:
The userData CLI argument seems to intend to set the user data directory, but the value passed is only used for loading packages, not for loading the preferences file. Preferences are still loaded from the default "Dynamo Core" folder.

When running the CLI, the Preferences singleton first gets initialized here, using the PathResolver created the line above, which is constructed without the userData folder from the cli args. Only a few lines later, when calling StartDynamoWithDefaultConfig, is the userData cli arg used, but by then Preferences has already been initialized.

This PR modifies the initial PathResolver above to make use of the userData parameter. If the preferences file does not exist at the userData folder it falls back to using the default user data folder for preferences (preserving current behavior).

Purpose

(FILL ME IN) This section describes why this PR is here. Usually it would include a reference to the tracking task that it is part or all of the solution for.

Declarations

Check these if you believe they are true

  • The codebase 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.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Fix a bug with setting preferences file location via userdata parameter in CLI.

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

Fall back to default if no preferences file present.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7409

Copy link

UI Smoke Tests

Test: success. 11 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests

PathManager.Instance.AssignHostPathAndIPathResolver(string.Empty, pathResolver);

if (!File.Exists(PathManager.Instance.PreferenceFilePath))
Copy link
Member

Choose a reason for hiding this comment

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

will this traverse the newly set user data folder from the command line args and check for a prefs file there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. If userData parameter is, e.g. %appData%\Dynamo\Dynamo Revit, PathManager.Instance.PreferenceFilePath is %appData%\Dynamo\Dynamo Revit\3.4\DynamoSettings.xml.

@mjkkirschner
Copy link
Member

can you say a bit more about this:
but not the preferences file location because of complexities in the initialization order.?

@twastvedt
Copy link
Contributor Author

can you say a bit more about this: but not the preferences file location because of complexities in the initialization order.?

Added some detail in the description, let me know if it's still unclear.

I'm struggling a bit to figure out how to test this. I can make a test which starts a separate CLI process with the userData argument, but I'm not sure how to find out what preference file was loaded in that process. I haven't found a preference setting I could set that would alter the result of a graph, without getting into a custom package. Any thoughts on a way to do this simply?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants