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 queue_name to VmOrTemplate::Operations::Snapshot queue methods #19616

Merged
merged 10 commits into from
Dec 11, 2019
Merged

Add queue_name to VmOrTemplate::Operations::Snapshot queue methods #19616

merged 10 commits into from
Dec 11, 2019

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented Dec 9, 2019

This PR adds a queue_name to the queue options for the VmOrTemplate::Operations::Snapshot#remove_snapshot_queue, VmOrTemplate::Operations::Snapshot#remove_evm_snapshot_queue and VmOrTemplate::Operations::Snapshot#remove_all_snapshots_queue mixin methods.

I've also added some specs (there were previously none for this mixin) and added some comments to those methods.

Part of #19543

@agrare
Copy link
Member

agrare commented Dec 9, 2019

@djberg96 the travis failure looks related

@djberg96
Copy link
Contributor Author

djberg96 commented Dec 9, 2019

@agrare I think I need a little help here. I'm not sure why it still fails after explicitly deleting the queue items.

@agrare
Copy link
Member

agrare commented Dec 10, 2019

@djberg96 the problem is the put_or_update find_options.

The MiqQueue record has :queue_name => "generic" and the find_options has :queue_name => nil so it is creating a duplicate.

You can change the ExtManagementSystem#queue_name_for_ems_operations to be "generic" instead of nil so it is more explicit.

@djberg96
Copy link
Contributor Author

@agrare Hm, I stubbed the ems to receive a custom queue name, but am still seeing the same failure.

@agrare
Copy link
Member

agrare commented Dec 10, 2019

@djberg96 that's not the spec that is failing

@agrare
Copy link
Member

agrare commented Dec 10, 2019

@djberg96 would prefer to change the core method rather than stub it in specs

Applied:

--- a/app/models/ext_management_system.rb
+++ b/app/models/ext_management_system.rb
@@ -606,7 +606,7 @@ class ExtManagementSystem < ApplicationRecord
   # Until all providers have an operations worker we can continue
   # to use the GenericWorker to run ems_operations roles
   def queue_name_for_ems_operations
-    nil
+    "generic"
   end

And that spec passes locally for me

@agrare
Copy link
Member

agrare commented Dec 11, 2019

I don't think you actually wanted that PR to close this :)

@agrare agrare reopened this Dec 11, 2019
@djberg96
Copy link
Contributor Author

@agrare Hah, no. I'll update the specs to get rid of the ems stub.

@agrare
Copy link
Member

agrare commented Dec 11, 2019

👍 awesome

@miq-bot
Copy link
Member

miq-bot commented Dec 11, 2019

Checked commits https://github.com/djberg96/manageiq/compare/be1a74441cab9efdc0979cde2dced074f9fe96b8~...f694f1e191c49720a7308ad1a8ae97e9d138c20e with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

agrare added a commit that referenced this pull request Dec 11, 2019
…name

Add queue_name to VmOrTemplate::Operations::Snapshot queue methods
@agrare agrare merged commit f694f1e into ManageIQ:master Dec 11, 2019
@agrare agrare added this to the Sprint 127 Ending Jan 6, 2020 milestone Dec 11, 2019
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.

3 participants