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 Openstack CinderManager EventCatcher #281

Merged
merged 2 commits into from
Apr 27, 2018

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Apr 26, 2018

No description provided.

@mansam mansam requested a review from aufi April 26, 2018 16:27
@mansam
Copy link
Contributor Author

mansam commented Apr 26, 2018

@aufi please review when you can, this is part of the fix for https://bugzilla.redhat.com/show_bug.cgi?id=1570965

@mansam mansam changed the title Add Openstack CinderManager EventCatcher [wip] Add Openstack CinderManager EventCatcher Apr 26, 2018
@miq-bot miq-bot added the wip label Apr 26, 2018
@JPrause
Copy link
Member

JPrause commented Apr 26, 2018

@miq-bot add_label blocker

@JPrause
Copy link
Member

JPrause commented Apr 26, 2018

@mansam if this can be backported, please add label gaprindashvili/yes

@mansam mansam force-pushed the add-openstack-cinder-event-catcher branch from 835a4cb to c4eb07f Compare April 26, 2018 17:57
@mansam mansam requested a review from Ladas April 26, 2018 17:58
@miq-bot
Copy link
Member

miq-bot commented Apr 26, 2018

Checked commits mansam/manageiq-providers-openstack@beb5f3d~...c4eb07f with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
8 files checked, 12 offenses detected

app/models/manageiq/providers/openstack/inventory/persister/target_collection.rb

app/models/manageiq/providers/openstack/storage_manager/cinder_manager.rb

app/models/manageiq/providers/openstack/storage_manager/cinder_manager/event_parser.rb

@mansam mansam changed the title [wip] Add Openstack CinderManager EventCatcher Add Openstack CinderManager EventCatcher Apr 26, 2018
@mansam mansam removed the wip label Apr 26, 2018
@JPrause
Copy link
Member

JPrause commented Apr 27, 2018

@Ladas since this is a blocker PR, can you review asap. We have a build coming up and are trying to get these blocker PRs merged.

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 good 👍

@mansam was there no event catcher before? Btw. in other providers we just have 1 events catcher for cloud,network and storage, to spare some workers. Maybe we could refactor it to this later?

@mansam
Copy link
Contributor Author

mansam commented Apr 27, 2018

@Ladas There wasn't as far as I could tell from the list of workers. I'd very much like to refactor down to a single event catcher when we've got a little more time to breathe. :)

@Ladas Ladas merged commit 4c97268 into ManageIQ:master Apr 27, 2018
@Ladas Ladas self-assigned this Apr 27, 2018
@Ladas Ladas added this to the Sprint 85 Ending May 7, 2018 milestone Apr 27, 2018
@simaishi
Copy link
Contributor

@mansam Yet another conflict on VCRs... I assume you'd need to create a separate PR for Gaprindashvili?

@mansam
Copy link
Contributor Author

mansam commented Apr 30, 2018

@simaishi Yes, if you're running into a VCR conflict I'll have to make a new PR.

@simaishi
Copy link
Contributor

Backported to Gaprindashvili via #283

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