Don't crash when probing deleted vApp for status #324
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
miha-plesko
force-pushed
the
dont-crash-when-probing-deleted-vapp
branch
from
51568da to
437e05d
Compare
|
Not what people usually say but thanks for not looking at this yesterday @agrare ;) Happened to notice we're GET-ing the vapp by ID in two places so I refactored a bit. Feel free to look at it now, I don't think I'll change my mind again :) |
With this commit we properly capture Fog exception which is raised when GET-ing vApp by ID when vApp doesn't exist anymore. Interesting enough, the fog-vcloud raises one of ```ruby Fog::VcloudDirector::Compute::Forbidden Fog::VcloudDirector::Compute::ServiceError ``` exceptions instead 404, depending on why exactly the entity couldn't be found :) With this commit we now capture the two exceptions and convert them to much more meaningful ``` MiqException::MiqOrchestrationStackNotExistError ``` to better reflect what's going on. Also, Automation is able to properly handle this kind of exception. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1632239 Signed-off-by: Miha Pleško <[email protected]>
miha-plesko
force-pushed
the
dont-crash-when-probing-deleted-vapp
branch
from
437e05d to
723d6ff
Compare
|
Checked commit miha-plesko@723d6ff with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@miha-plesko haha I do what I can |
agrare
approved these changes
Sep 27, 2018
agrare
added a commit
that referenced
this pull request
Sep 27, 2018
…ted-vapp Don't crash when probing deleted vApp for status
|
@miq-bot add_label blocker |
simaishi
pushed a commit
that referenced
this pull request
Oct 1, 2018
…ted-vapp Don't crash when probing deleted vApp for status (cherry picked from commit ab9549d) https://bugzilla.redhat.com/show_bug.cgi?id=1632239
|
Hammer backport details: |
simaishi
pushed a commit
that referenced
this pull request
Oct 11, 2018
…ted-vapp Don't crash when probing deleted vApp for status (cherry picked from commit ab9549d) https://bugzilla.redhat.com/show_bug.cgi?id=1636494
|
Gaprindashvili backport details: |
simaishi
added a commit
that referenced
this pull request
Oct 11, 2018
…ing-deleted-vapp" This reverts commit ff0e5f4. https://bugzilla.redhat.com/show_bug.cgi?id=1636494
|
Reverted the G backport. |
|
@simaishi why would you revert it? |
|
@miha-plesko This was backported only because it was tied to https://bugzilla.redhat.com/show_bug.cgi?id=1636494. Since that's no longer needed in Gaprindashvili branch, I needed to revert all related PRs. We're not backporting any PRs unless it's associated to a BZ that's approved for the next release... |
agrare
pushed a commit
to agrare/manageiq-providers-vmware
that referenced
this pull request
Apr 15, 2019
Tag migrated VM.
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.
With this commit we properly capture Fog exception which is raised when GET-ing vApp by ID when vApp doesn't exist anymore. Interesting enough, the fog-vcloud raises one of
exceptions instead 404, depending on why exactly the entity couldn't be found :)
With this commit we now capture the two exceptions and convert them to much more meaningful
to better reflect what's going on. Also, Automation is able to properly handle this kind of exception.
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1632239
Related PRs:
@miq-bot add_label bug,gaprindashvili/yes,hammer/yes
@miq-bot assign @agrare