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

DM-37836: bug fixes for ctrl_execute following initial testing #18

Merged
merged 1 commit into from Feb 3, 2023

Conversation

daues
Copy link
Contributor

@daues daues commented Feb 2, 2023

…ormatting

@mxk62 mxk62 changed the title one line change to give dynamic option default value, not None; and f… DM-37836: bug fixes for ctrl_execute following initial testing Feb 2, 2023
@@ -143,7 +143,7 @@ def parseArgs(self, basename):
action="store",
dest="dynamic",
type=str,
default=None,
default="__default__",
Copy link

Choose a reason for hiding this comment

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

No objections regarding changing the default value, but the command line help below is slightly misleading. Based on the current description I initially assumed that the --dynamicoption is a simple on/off flag, but looking at the code I see that one can actually use it to pass a custom template for configuring some aspects of the dynamic slot. So it would be nice if the help was reflecting that, something like:

a template file to use when configuring dynamic/partitionable slot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the "file input" for --dynamic, I will want to do some testing as I have not touched that for a while. Plus it ties together with offering memory control in Slurm, so I will do those together at some point, before advertising the feature more loudly.

@daues daues merged commit 5c1d445 into main Feb 3, 2023
@daues daues deleted the tickets/DM-37836 branch February 3, 2023 03:45
@daues daues restored the tickets/DM-37836 branch February 3, 2023 03:46
@timj timj deleted the tickets/DM-37836 branch February 7, 2023 21:25
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.

None yet

2 participants