-
Notifications
You must be signed in to change notification settings - Fork 9
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
Support for reset/image/load activation profiles #499
Conversation
20e5bf5
to
84bd01d
Compare
725c2a8
to
9838732
Compare
f242df9
to
cf472aa
Compare
9a5a93a
to
e3782f4
Compare
e3782f4
to
58d14be
Compare
d116ffe
to
492d93f
Compare
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.
Note: this is so much code that I just scrolled over some parts.
I think you could significantly reduce line count just by introducing helper methods for a lot of repeated structures, that do always the same, only using different property name strings for example.
492d93f
to
fad7ba3
Compare
I have introduced helper functions for the setting of properties from specially handled options. Please re-review. |
fad7ba3
to
347b56b
Compare
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 better now.
zhmccli/_cmd_imageprofile.py
Outdated
} | ||
|
||
org_options = original_options(options) | ||
if org_options['copy-name'] is not None: |
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 wonder whether it is a good idea to copy the properties from the default profile ourselves (i.e. on the client side) as opposed to letting that be done by the HMC. I think my motivation to copy it on the client side was that we would be on the safe side given that we are not representing all resource properties 1:1 as command options, but I don't think initializing the properties from the default profile is necessary for that to work.
Also, since the HMC uses the profile named "DEFAULT" when --copy-name is not specified, it is inconsistent to copy the profile when --copy-name is specified, but not to copy the "DEFAULT" profile when it is not specified.
Overall, I suggest that we do not initialize the properties from the default profile, and let the HMC do that.
Applies to all three activation profile types.
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 removed the client-side copying.
Please review that approach.
@EdGue42
I looked again at the So we could remove the support for a Cpc resource object in the find_:::() functions and reduce that to just always the CPC name. That would eliminate some other portions of duplicated code. I'll do that in a separate PR. Update: That became PR #525 |
347b56b
to
26efa08
Compare
6217248
to
d64a812
Compare
73b0398
to
98fdf7a
Compare
Details: * Added command group 'resetprofile' for operations on reset activation profiles in classic mode CPCs. * Added command group 'imageprofile' for operations on image activation profiles in classic mode CPCs. * Added command group 'loadprofile' for operations on load activation profiles in classic mode CPCs. Signed-off-by: Andreas Maier <[email protected]>
98fdf7a
to
8222fae
Compare
Ready for review.
Has been tested on A65.
To try this PR yourself, run
make install
after checking out the branch, in order to install the updated "zhmcclient" package.