Skip to content
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

Raise Miq exceptions on connect #162

Merged
merged 1 commit into from Nov 30, 2017

Conversation

borod108
Copy link
Contributor

The raw_connect method of providers is intended to check connection and authentication details.
This method should report errors throwing instances of MiqEVMLoginError, MiqInvalidCredentialsError, etc.

This fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1510374

@borod108
Copy link
Contributor Author

@jhernand will you please take a look.

rescue
_log.error("Error while verifying credentials #{$ERROR_INFO}")
raise MiqException::MiqEVMLoginError, $ERROR_INFO
end

def rethrow_as_a_miq_error(e)
if e.message.include?("The username or password is incorrect")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to write here a comment (maybe a TODO) explaining that in version 4.2 of the SDK there will be specific exceptions for these cases. Once we update to 4.2 this should be modified to use those exceptions instead of this fragile string checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will create a git issue once this is merged, I think its easier to track than a TODO in the code.
Also added a trello card in backlog.

@borod108
Copy link
Contributor Author

@borod108
Copy link
Contributor Author

borod108 commented Nov 30, 2017

please do not merge yet
@miq-bot add-lable wip

@miq-bot
Copy link
Member

miq-bot commented Nov 30, 2017

@borod108 unrecognized command 'add-lable', ignoring...

Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

The `raw_connect` method of providers is intended to check connection and authentication details.
This method should report errors throwing instances of `MiqEVMLoginError`, `MiqInvalidCredentialsError`, etc.

This fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1510374
@miq-bot
Copy link
Member

miq-bot commented Nov 30, 2017

Checked commit borod108@4a70f08 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@borod108
Copy link
Contributor Author

@masayag will you take a look and merge?

@masayag masayag merged commit 7d33f02 into ManageIQ:master Nov 30, 2017
@masayag
Copy link
Contributor

masayag commented Nov 30, 2017

@miq-bot add_labels bug, Gaprindashvili/yes

simaishi pushed a commit that referenced this pull request Dec 1, 2017
@simaishi
Copy link
Contributor

simaishi commented Dec 1, 2017

Gaprindashvili backport details:

$ git log -1
commit c04deaab29df3a3818725a6cbced02b69b0f8fbd
Author: Moti Asayag <masayag@redhat.com>
Date:   Thu Nov 30 16:21:37 2017 +0200

    Merge pull request #162 from borod108/bugs/1510374miqerror
    
    Raise Miq exceptions on connect
    (cherry picked from commit 7d33f02c2e3520a2c721a2a218b6f1ec32a4732d)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1519815

@borod108
Copy link
Contributor Author

borod108 commented Dec 3, 2017

1 similar comment
@borod108
Copy link
Contributor Author

borod108 commented Dec 3, 2017

@miq-bot
Copy link
Member

miq-bot commented Dec 3, 2017

@borod108 unrecognized command 'add-lable', ignoring...

Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

@agrare agrare added this to the Sprint 75 Ending Dec 11, 2017 milestone Dec 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants