-
Notifications
You must be signed in to change notification settings - Fork 4
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
Ability to Upload Scheduling Goals to All Plans with Mission Model ID #70
base: develop
Are you sure you want to change the base?
Conversation
Hotfix: Windows Paths
src/aerie_cli/commands/scheduling.py
Outdated
plan_id: int = typer.Option( | ||
..., help="Plan ID", prompt=True | ||
help="Plan ID (optional)", default=-1, prompt=True #how to make this optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the user still be prompted for the plan-id if it's now optional?
Right now, I still have it prompting the user for the plan-id, but if the user does not input anything and skips past it, then the default value is -1 (not a valid plan-id).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to set prompt
to False
. IMO, the intuitive interface here would be to specify either the Plan ID or a Model ID, but not both, and throw an error if the user specifies both. If you want to preserve some sort of prompts, you can manually implement the logic with a menu (i.e., prompt the user to select whether they upload to a single plan or all plans for a given model). I have a utility that should make this easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the Model ID is used within the the scheduling upload object, so I'm not sure if model id still needs to be an input when there is also a plan id. I could also query the model ID from the plan ID in order to assert that both are not inputted.
I will still make a selection menu so the user knows the two options and set both model id and plan id prompts to False.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point -- yeah, you should be able to get the model ID from the plan ID (using the same list_all_activity_plans
method as below, if you want).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to not throw an error when both model id and plan id are inputted. I added a check if the model id is not inputted, then I find the model id from list_all_activity_plans
.
I’m also making a couple checks to ensure the inputs that are needed are there using the select_from_list
method. I set the schedule prompt to False to keep it consistent with the other inputs so they all inputs are prompted together.
Since there are already so many queries in this class, I might suggest you use an existing query instead of adding for plan in filter(lambda p: p.model_id == model_id, aerie_client.list_all_activity_plans()):
plan_id = plan.id
# etc. |
Addresses #69
Updated scheduling "upload" method such that if you do not have to specify the plan-id, the scheduling goals will upload to each plan in the specified model.