Fix messaging_client exception if no ENV and no yaml are present #20062
Conversation
If the messaging_type is set to artemis or kafka but no ENV vars for messaging are set and no config/messaging.yml file is created the messaging_client_options method throws a NilClass exception.
|
Checked commit agrare@8aa71e1 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint |
| @@ -47,7 +47,10 @@ def self.messaging_client(client_ref) | |||
| # caching the client works, even if the connection becomes unavailable | |||
| # internally the client will track the state of the connection and re-open it, | |||
| # once it's available again - at least thats true for a stomp connection | |||
| ManageIQ::Messaging::Client.open(messaging_client_options.merge(:client_ref => client_ref)) | |||
| options = messaging_client_options&.merge(:client_ref => client_ref) | |||
| return if options.nil? | |||
There was a problem hiding this comment.
If you move this line up, then you don't need the Sorry misread the diff.&.merge
There was a problem hiding this comment.
Should we also log an error message? Something along the lines of "Messaging type set to #{messaging_type}, but no configuration provided, falling back to miq_queue"
There was a problem hiding this comment.
Without some rate limiting that would be a log message for every event and metric that gets raised
There was a problem hiding this comment.
I think when we are depending on the Messaging::Client connection we should raise an exception if the config is missing, right now it is just "in addition to" so wanted to keep the noise to a minimum
| :encoding => "json", | ||
| :protocol => messaging_protocol, | ||
| ).tap { |h| h[:password] = MiqPassword.try_decrypt(h.delete(:password)) } | ||
| )&.tap { |h| h[:password] = MiqPassword.try_decrypt(h.delete(:password)) } |
There was a problem hiding this comment.
Does it make sense for this method to return an empty hash instead of nil?
|
Minor side comments, but otherwise LGTM |
…_yml Fix messaging_client exception if no ENV and no yaml are present (cherry picked from commit d16c6e7)
|
Jansa backport details: |
If the messaging_type is set to artemis or kafka but no ENV vars for messaging are set and no config/messaging.yml file is created the messaging_client_options method throws a NilClass exception.
WARN -- : MIQ(MiqQueue.messaging_client) Failed to open messaging client: undefined method 'merge' for nil:NilClass