Add raw_connect? method to ExtManagementSystem #16636
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
From OpenStack provider point of view, LGTM! |
jntullo
force-pushed
the
raw_connect!
branch
from
ca293dc to
88b4cd1
Compare
Fryguy
reviewed
Dec 12, 2017
app/models/ext_management_system.rb
Outdated
| @@ -217,6 +217,10 @@ def self.create_discovered_ems(ost) | |||
| end | |||
| end | |||
|
|
|||
| def self.raw_connect!(*params) | |||
| !!raw_connect(*params) | |||
There was a problem hiding this comment.
My only issue is that ! as a suffix usually implies an Exception or side effect could occur. I'm thinking this should be raw_connect? instead, which implies it will return a boolean.
jntullo
force-pushed
the
raw_connect!
branch
from
88b4cd1 to
5c165e8
Compare
jntullo
changed the title
When validating credentials on the queue, existing raw_connect methods return the actual connection, which is then being placed into the logs on callback. It should not be present in the logs. The credential validation only cares about a success or failure, not the connection, so this raw_connect? wrapper method will return a true value on successful validation, rather than the connection itself.
jntullo
force-pushed
the
raw_connect!
branch
from
5c165e8 to
835b837
Compare
|
Checked commit jntullo@835b837 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
simaishi
pushed a commit
that referenced
this pull request
Dec 14, 2017
Add raw_connect? method to ExtManagementSystem (cherry picked from commit 13ce272)
|
Gaprindashvili backport details: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When validating credentials on the queue, existing raw_connect methods return the actual connection, which is then being placed into the logs on callback. It should not be present in the logs. The credential validation only cares about a success or failure, not the connection, so this raw_connect? wrapper method will return a true value on successful validation, rather than the connection itself.
Log Before where you will see
#<Aws::EC2::Resource>in the result:Log after where you will only see true:
Resolves ManageIQ/manageiq-providers-openstack#174
@miq-bot add_label gaprindashvili/yes, bug
@miq-bot assign @Fryguy
cc: @aufi @Ladas @dikonoor