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

Fix RefreshWorker before_exit arguments #138

Merged
merged 1 commit into from
Nov 21, 2017

Conversation

agrare
Copy link
Member

@agrare agrare commented Nov 16, 2017

The before_exit method takes two arguments, message and exit code. The
method in the vmware refresh_worker didn't take any arguments and led to
this exception when shutting down:

MIQ(ManageIQ::Providers::Vmware::InfraManager::RefreshWorker::Runner) ID [25] PID [9366] GUID [4e2d7762-cbb8-486f-aeb7-fb441c08aaea] Error in before_exit: wrong number of arguments (given 2, expected 0)

This would only cause an issue if you were using the update driven
refresh (not default) and in that case the thread would be terminated
instead of cleanly shut down.

The before_exit method takes two arguments, message and exit code.  The
method in the vmware refresh_worker didn't take any arguments and led to
this exception when shutting down:

```
MIQ(ManageIQ::Providers::Vmware::InfraManager::RefreshWorker::Runner) ID [25] PID [9366] GUID [4e2d7762-cbb8-486f-aeb7-fb441c08aaea] Error in before_exit: wrong number of arguments (given 2, expected 0)
```

This would only cause an issue if you were using the update driven
refresh (not default) and in that case the thread would be terminated
instead of cleanly shut down.
@miq-bot
Copy link
Member

miq-bot commented Nov 16, 2017

Checked commit agrare@47300e2 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. 🍪

@agrare
Copy link
Member Author

agrare commented Nov 20, 2017

cc @blomquisg

@Ladas Ladas self-assigned this Nov 21, 2017
Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

Looks great

@Ladas Ladas merged commit 64720ef into ManageIQ:master Nov 21, 2017
@agrare agrare deleted the fix_refresh_worker_before_exit branch November 21, 2017 18:39
simaishi pushed a commit that referenced this pull request Nov 27, 2017
Fix RefreshWorker before_exit arguments
(cherry picked from commit 64720ef)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit d6af0ee1bc1711c5ba8444f3e21db419a342a9d2
Author: Ladislav Smola <[email protected]>
Date:   Tue Nov 21 18:18:00 2017 +0100

    Merge pull request #138 from agrare/fix_refresh_worker_before_exit
    
    Fix RefreshWorker before_exit arguments
    (cherry picked from commit 64720efdb6a80fc69eefff7f394678d92a9c3f39)

@agrare agrare added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 27, 2017
agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Apr 15, 2019
…olumes

Added cloud volumes to requested resource calculation.
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