Add development and testing option to permit installing unsupported PE versions #204
Conversation
|
|
There was a problem hiding this comment.
Thanks for the PR @jarretlavallee!
I'd like to request some minor changes.
- Please rename
peadm::install::enable_unsafe_versionstopeadm::install::permit_unsafe_versions. I think "permit" better describes what is going on here compared to "enable". - Please use the same parameter name,
permit_unsafe_versions, in the function - it isn't as meaningful there, but it would be good for consistency. - In the function, please change the warning text "Enabling" to "Permitting".
Thanks!!
Prior to this commit, there was no way to force an override to the PE version assertion. This commit adds a new parameter that will override the PE version check and output a warning when enabled. This is useful in testing new PE versions that are not yet supported by the module.
jarretlavallee
force-pushed
the
sup_2643_version_override
branch
from
c6572e5 to
d98c551
Compare
|
@reidmv Sorry for the delayed response as I somehow missed the email. Thank you for taking the time to review this. I made the changes as you described. |
There was a problem hiding this comment.
Thanks @jarretlavallee!
I just realized looking at the specs that what should happen here when $permit_unsafe_versions = true is, the assertion should not fail the plan, but the return data should actually state { supported => false }.
If you have time to make that tweak I think that'll make this ready to merge. If not, I can probably sneak it in soon myself, after which we can merge it.
Lemme know if you start working on that change. I'll do the same; just wanna avoid duplicating effort :D
|
|
||
| context 'unsafe versions' do | ||
| it 'accepts PE versions that are too old' do | ||
| is_expected.to run.with_params('2018.1.0', true).and_return({ 'supported' => true }) |
There was a problem hiding this comment.
Ah, shoot. Just realized that the intuitive result here is that the assert function does NOT raise (good so far), but the return value should actually be { 'supported' => false }.
The new $permit_unsafe_versions parameter allows the assert_supported_pe_version() function to not raise on failure, but it should always return accurate data about whether or not the given version is or isn't supported.
reidmv
changed the title
Prior to this PR, there was no way to force an override to the PE version assertion. This PR adds a new parameter that will override the PE version check and output a warning when enabled. This is useful in testing new PE versions that are not yet supported by the module.
Note that the installation plan will emit two copies of the warning due to the function call in the
installandsubplan::installplans. I did not correct this as it seems acceptable to emit the warning multiple times.