Fix StorageManagers Cross Linkers #14795
Conversation
Fix Cinder and Swift Cross Linkers to make sure valid data is available before processing. Cinder should have Volumes, Backups, or Snapshots. Swift must have Object Storage. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1441144.
|
Checked commit jerryk55@3c1c6b9 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
| @@ -5,6 +5,10 @@ def self.cross_link(ems, data) | |||
| _log.warn "Manager does not have a parent." | |||
| return | |||
| end | |||
| unless data | |||
| _log.warn "Manager does not have volumes, snapshots, or volume backups." | |||
There was a problem hiding this comment.
We usually require parenthesis around method arguments.
There was a problem hiding this comment.
Just duplicating the previous code. Are you asking for me to go back and change existing code, or to have the new code be inconsistent with the old code?
| @@ -5,6 +5,10 @@ def self.cross_link(ems, data) | |||
| _log.warn "Manager does not have a parent." | |||
| return | |||
| end | |||
| unless data | |||
| _log.warn "Manager does not have any object storage." | |||
There was a problem hiding this comment.
And just for perspective, under manageiq/app/models - there are 1373 calls to log methods using "(" and 642 not using them. It is not consistent although there is a 2/1 ratio for.
There was a problem hiding this comment.
That's why I made it just a comment, not request changes. We seem to be losing consistency on this one since the rubocop doesn't complain about it.
There was a problem hiding this comment.
We seem to be losing consistency on this one since the rubocop doesn't complain about it.
This is troubling since it should be complaining.
There was a problem hiding this comment.
@Fryguy See http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Lint/RequireParentheses
They only seem concerned about having parenthesis if there is && or ||. 😢
Fix Cinder and Swift Cross Linkers to make sure valid data is available before
processing. Cinder should have Volumes, Backups, or Snapshots. Swift must
have Object Storage.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1441144.
@roliveri @hsong-rh @Fryguy please review. Merge when appropriate.
Steps for Testing/QA
Add an Openstack cloud provider that does not have any objects (for Swift) or volumes, backups, or snapshots (for Cinder). Do a refresh of the provider after adding it. The refresh should not result in the issue shown in the above Bugzilla.