-
Notifications
You must be signed in to change notification settings - Fork 39
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
Refs #12266 - fixing case where no certs exist #86
Conversation
@@ -43,7 +43,8 @@ def last_rpm | |||
{'release' => release, 'rpm' => rpm} | |||
end | |||
|
|||
rpms.sort { |a,b| a['release'].to_i <=> b['release'].to_i }.last['rpm'] | |||
rpm = rpms.sort { |a,b| a['release'].to_i <=> b['release'].to_i }.last | |||
rpm['rpm'] if rpm |
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 this will cause an error above - when we make the symlink on L25. Probably need to gate that with a last_rpm.blank?
check
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.
based on what it used to be: https://github.com/Katello/puppet-certs/pull/83/files it seems like it handles nil okay, but i can skip up above if its nill to be more explicit.
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.
actually after testing File.symlink() does not like nil. How was this working before? very strange
@stbenjam updated |
tested and this seemed to get past this error, but other errors still occured |
@@ -20,9 +20,9 @@ def run | |||
'--requires', 'subscription-manager', | |||
'--post', post_script_file, | |||
*resource[:files]) | |||
if resource[:alias] | |||
if rpm = last_rpm && resource[:alias] |
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.
Nitpick, but usually assignment in a conditional should be in parens. Do we enforce that?
Otherwise good for me, were the additional errors still related to this? Or something else? From a code perspective I think this change is fine, but didn't test yet. |
undefined method [] for nil:NilClass /usr/share/katello-installer-base/modules/certs/lib/puppet/provider/certs_bootstrap_rpm/katello_ssl_tool.rb:46:in last_rpm
@stbenjam updated with parens. The errors were unrelated, likely related to theforeman/puppet-foreman#436 |
ACK |
undefined method [] for nil:NilClass
/usr/share/katello-installer-base/modules/certs/lib/puppet/provider/certs_bootstrap_rpm/katello_ssl_tool.rb:46:in last_rpm