-
Notifications
You must be signed in to change notification settings - Fork 52
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
cmd: add deployment resource start/start-maintenance commands #89
Conversation
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.
Looks good
|
||
// Validate ensures the parameters are usable by Start. | ||
func (params StartParams) Validate() error { | ||
var merr = new(multierror.Error) |
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 think we have a common structure right @marclop to avoid duplication?
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.
Do you mean depresource.NewStateless{}
? I don't think that struct works very well here, It has a bunch of fields that aren't relevant, and also the Validate()
and fillDefaults()
methods are different.
I can make a common struct for these commands and stop
/stop-maintenance
called StopStartParams
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.
No , no , I can't find a discussion I had with Marc last week on a GH pr about the same. @marclop can u help pls? :)
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.
We discussed out of band that going forward probably a common structure with: API
, DeploymentID
, RefID
, Type
can be defined and used with all the validations in common. Including the fillDefaults ref-id auto-discovery.
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.
Ok so there will be another PR introducing the common struct with validation + filldefaults.
Great. we are good to go then
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.
Awesome
Description
Adds the following command:
ecctl deployment resource start
which starts a previously stopped deployment resource.ecctl deployment resource start-maintenance
which starts maintenance mode on a deployment resource.Related Issue
#83
Motivation and Context
Implementing more of the deployment/resource APIs.
How Has This Been Tested?
Manually against an ece environment and by running unit test suite.
Types of Changes
Readiness Checklist