-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Switch to using qpid bind #177
Conversation
Relies on theforeman/puppet-qpid#52 |
manifests/qpid.pp
Outdated
ssl_cert => $client_cert, | ||
queue => $candlepin_event_queue, | ||
exchange => 'event', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not ideal to hardcode, but did that for now since this should get more re-factoring as we split things out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could introduce a $candlepin_event_exchange
and hardcode the default there. That at least makes it easier to override later on.
Updated to add a variable for the candlepin exchange as a placeholder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also refactor the qpid-config execs to use qpid::config_cmd
?
manifests/qpid.pp
Outdated
@@ -25,8 +26,9 @@ | |||
require => Service['qpidd'], | |||
logoutput => true, | |||
} ~> | |||
qpid::bind_event { ['entitlement.created', 'entitlement.deleted', 'pool.created', 'pool.deleted', 'compliance.created']: | |||
qpid::bind { ['entitlement.created', 'entitlement.deleted', 'pool.created', 'pool.deleted', 'compliance.created']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should now also be qpid::config::bind
030005a
to
8f8def5
Compare
Refactored! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much easier to read!
manifests/qpid.pp
Outdated
ssl_cert => $client_cert, | ||
} ~> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a simple ->
I think.
Updated per comments |
Since theforeman/puppet-qpid#52 is now merged, could you rebase to see if the tests pass? |
@ekohl that did the trick, thanks. |
No description provided.