Add a commandLine flag for specifying geometryLib path #9949
Conversation
This file contains 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
remove RSA (need to use flag) add flag for passing ASM path to command line args add overload to use flag when present to sandbox,cli,cliwpf
mjkkirschner
changed the title
|
Does this version include the recursive product lookup or are you going to put that back in by (revert*4)ing the other PR? |
this PR includes the recursive product lookup, will have to make sure it keeps it when cherry picking. |
refactor for testing modify docs from review
|
PTAL - this is ready for another review - tested by starting sandbox from a random folder full of asm binaries and made sure asm worked and was loaded from there. See image above. |
mjkkirschner
changed the title
mjkkirschner
changed the title
QilongTang
reviewed
Aug 30, 2019
QilongTang
reviewed
Aug 30, 2019
sandbox should still start even if path is invalid asm path
|
all tests pass on self serve. |
QilongTang
reviewed
Sep 5, 2019
QilongTang
reviewed
Sep 5, 2019
QilongTang
reviewed
Sep 5, 2019
|
A few comments then LGTM |
|
LGTM |
QilongTang
changed the title
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.
add flag for passing ASM path to command line args
add overload to use flag when present to sandbox, cli, cliwpf
Purpose
Attempt to allow integrators to use DynamoSandbox without requiring potentially slow recursive file search on the users system.
If this PR is merged the consequences are:
sandbox daily builds will not load on applications which do not have ASM in their root directory - this is the same behavior as 2.3 (even if they are added to the list to search for)will need to usecan use a command line flag --gpto pass the geometryPath to sandbox to have it skip searching and load that directly.a thought:
to enable the most flexible solution - we can keep the recursive loading but move our tests and other integrators to use the command line flag (or preload option) to avoid searching - for example - we can modify our tests to stop geometry library searching and instead use this same mechanism to load the geometry library from a known a specific location.This will enable us to keep the slow searching behavior for daily build users but skip it for integrators and tests...
This PR now simply adds the commandLineFlag and keeps the behavior @aparajit-pratap added. So both can used for most flexibility depending on user type.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@QilongTang
@aparajit-pratap
FYIs
@ColinDayOrg