-
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
Open
zgoldberg22
wants to merge
15
commits into
develop
Choose a base branch
from
upload-goals-to-all-plans
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
16ef72b
Merge pull request #57 from NASA-AMMOS/hotfix--windows-paths
cartermak 5fbc619
added to scheduling upload method to upload schedling goals to all pl…
7276aa4
fixes to query error
zgoldberg22 3d4819b
change default of plan_id in upload function
zgoldberg22 685af48
set prompt back to true
zgoldberg22 aa24087
fix query for all plans in model id and update upload method
zgoldberg22 7093d27
added test command with methods to test query
zgoldberg22 40305e5
query response fix
zgoldberg22 43b959b
delete unnecessary code, fixed upload key id error
zgoldberg22 7d4d38f
fixed scheduling unique goal id error
zgoldberg22 53f77aa
delete query, update user prompts
zgoldberg22 89dd275
added input checks
zgoldberg22 692e2c5
conversion type error fix
zgoldberg22 5825e9f
resolve merge conflicts with develop branch
zgoldberg22 0d82a60
Merge branch 'develop' into upload-goals-to-all-plans
cartermak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,4 +111,4 @@ def list(): | |
) | ||
|
||
console = Console() | ||
console.print(table) | ||
console.print(table) |
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
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
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.
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
toFalse
. 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.