sandialabs / sceptre-phenix

phenix is an orchestration tool and GUI for Sandia's minimega platform
https://sandialabs.github.io/sceptre-docs/
GNU General Public License v3.0
17 stars 23 forks source link

Disable scenario apps during experiment creation #148

Closed causand22 closed 11 months ago

causand22 commented 1 year ago

This PR allows a user to disable applications from a scenario when creating an experiment. Instead of having to create a separate scenario file and go through the process of setting up phenix again, the user can live-make a custom scenario that is a subset of an existing scenario. The tags that appear when a user selects a scenario on the 'create an new experiment' window can be toggled to disable / enable apps.

Changes:

TODO:

There would be an issue if two different applications in a scenario shared the same name. Not sure if that is possible.

activeshadow commented 1 year ago

@causand22 thanks for submitting this! I'll try to do an initial review ASAP.

causand22 commented 1 year ago

@activeshadow If it is possible for a phenix scenario to have two apps that have the same name but different settings -- I have a fix for that. Let me know and I can merge it in to the branch. Instead of sending back a list of app names we'd send back a list of enabled/disabled flags. It might simplify things.

activeshadow commented 1 year ago

@causand22 phenix does not support the same app more than once in a scenario.

activeshadow commented 1 year ago

@causand22 I have not had a chance to review yet... sorry. Will try to ASAP.

From your perspective, is this ready to go and you're just waiting on a review, or do you know of more work you'd still like to do on it?

causand22 commented 1 year ago

@activeshadow No worries! I think it's ready to go -- maybe with some UI color changes. There might be a better way to disable the apps too, if the way I wrote isn't satisfactory.

There's some discussion of changing how this works -- instead of essentially deleting the apps on experiment creation, we disable the app on the first run. That way when an experiment is running you can click on the list of apps and re-run a previously disabled one. I think this would take a lot more effort to implement -- I haven't looked too much into the code but I'd imagine it would be more complicated than deleting at creation. Might be worth thinking about in the future, but I'm not sure I can get it done by the timeline.

All that to say - I think it's ready for review. Not sure I can implement additional features in a reasonable time.

activeshadow commented 1 year ago

@causand22 what if we added a disabled key to the app schema that defaults to false but can be set to true. In the code that processes the scenario apps, we could add a check for disabled on stages that are run automatically but ignore it when an app's running stage is triggered manually?

causand22 commented 1 year ago

@activeshadow That would work. I'll take a crack at it today.

mgaliar commented 1 year ago

@causand22 I did a few tests...Creation of an experiment from the cli including a scenario file seems to not apply the apps in the scenario file. Creating the same experiment through the UI does seem to apply the apps.

mgaliar commented 1 year ago

That fixed it!

causand22 commented 12 months ago

Made a few changes with the recent commits. Here's what's included:

activeshadow commented 12 months ago

Nice. Thanks @causand22!

@mgaliar can you test one more time with the latest changes?

mgaliar commented 11 months ago

Looks good to me!

causand22 commented 11 months ago

@activeshadow Anything else I need to add / change / fix?

activeshadow commented 11 months ago

Sorry, been busy. I'll do my best to test this and get it merged tomorrow.

activeshadow commented 11 months ago

@causand22 everything looks good except that I don't see a way to disable apps for experiments created via the command line.

causand22 commented 11 months ago

@activeshadow I'll add that now

causand22 commented 11 months ago

@activeshadow added feature + did some quick tests. Using the --disabled-apps "app1,app2" flag on command line during experiment creation should have the same result as disabling through GUI