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

Add more robust error handling to ConversionHost#get_conversion_state #18258

Merged
merged 6 commits into from
Mar 27, 2019
Merged

Add more robust error handling to ConversionHost#get_conversion_state #18258

merged 6 commits into from
Mar 27, 2019

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented Dec 3, 2018

Based on a conversation with Kedar Kulkarni, this PR attempts to add some more robust error handling to the get_conversion_state method. At the moment the SFTP download is sometimes failing somehow, so the data stream ends up being blank, which then results in a JSON parser error. This doesn't tell us the "real" error, which I'm hoping to figure out.

Original reported at https://bugzilla.redhat.com/show_bug.cgi?id=1653407 (but don't use this)

(The other micro changes you see were already-existent blank spaces that were auto-stripped by my editor, I'm surprised rubocop didn't catch those before).

WIP for now as I'm not sure that this will be enough, or if MiqSshUtil (from gems-pending) needs some TLC.

https://bugzilla.redhat.com/show_bug.cgi?id=1695847

@djberg96 djberg96 changed the title [WIP] Add more robust error handling to ConversionHost#get_conversion_state. [WIP] Add more robust error handling to ConversionHost#get_conversion_state Dec 3, 2018
@kedark3
Copy link

kedark3 commented Dec 3, 2018

@djberg96 thanks. 👍

@miq-bot miq-bot added the wip label Dec 3, 2018
@miq-bot
Copy link
Member

miq-bot commented Mar 26, 2019

This pull request is not mergeable. Please rebase and repush.

@djberg96 djberg96 changed the title [WIP] Add more robust error handling to ConversionHost#get_conversion_state Add more robust error handling to ConversionHost#get_conversion_state Mar 26, 2019
@djberg96
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Mar 26, 2019
@djberg96
Copy link
Contributor Author

@miq-bot add_label transformation, refactoring

@djberg96
Copy link
Contributor Author

@miq-bot remove_label unmergeable

@miq-bot
Copy link
Member

miq-bot commented Mar 27, 2019

Checked commits https://github.com/djberg96/manageiq/compare/a9f82feeb177c29aeba9f86d348f12d9f59b6bc8~...e7482f1795c7d8afa8313d268b744971cbec3cd1 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@agrare agrare self-assigned this Mar 27, 2019
@agrare agrare merged commit f73e7e6 into ManageIQ:master Mar 27, 2019
@agrare agrare added this to the Sprint 108 Ending Apr 1, 2019 milestone Mar 27, 2019
@ghost
Copy link

ghost commented Apr 3, 2019

@miq-bot add-label hammer/yes

simaishi pushed a commit that referenced this pull request Apr 22, 2019
Add more robust error handling to ConversionHost#get_conversion_state

(cherry picked from commit f73e7e6)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1702055
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 7709b441d46250f2f5ac08ba6918b78bbfd967be
Author: Adam Grare <agrare@redhat.com>
Date:   Wed Mar 27 09:06:03 2019 -0400

    Merge pull request #18258 from djberg96/get_conversion_state
    
    Add more robust error handling to ConversionHost#get_conversion_state
    
    (cherry picked from commit f73e7e68382dd99b295c56b4409b4c0a4118c7a5)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1702055

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.

5 participants