-
Notifications
You must be signed in to change notification settings - Fork 120
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
Adding log_and_notify method into log_object embedded method. #423
Conversation
@miq-bot add_reviewer @tinaafitz |
handle.log(log_level(level.downcase.to_s), message) | ||
end | ||
|
||
def self.log_level(level) |
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.
@billfitzgerald0120 What is this trying to do, does notify use warning instead of warn, if the caller is passing in level couldn't we enforce that the level be "info","warn",or"error"
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.
@mkanoor yes, notify uses warning and the log uses warn. The notify accepts info, warning,
error and success. The log accepts info, warn and error. For level, If a invalid value like 'fred' is passed in then it is changed to 'info' for both notify and log.
%w(info warn error).include?(log_level) ? log_level : "info" | ||
end | ||
|
||
def self.notify_level(level) |
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.
@billfitzgerald0120 Instead of using a function can we just use a hash e.g
NOTIFY_LEVEL_TO_LOG_LEVEL = {
'info' => 'info',
'warning' => 'warn',
'error' => 'error',
'success' => 'info'
}
NOTIFY_LEVEL_TO_LOG_LEVEL.default = 'info'
To fetch values you could use something like
puts NOTIFY_LEVEL_TO_LOG_LEVEL['fred']
puts NOTIFY_LEVEL_TO_LOG_LEVEL['warning']
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.
@mkanoor Do you want me to change this also?
def self.notify_level(level)
notify_level = level == "warn" ? "warning" : level
%w(info warning error success).include?(notify_level) ? notify_level : "info"
end
# If you want to create a notification and log a message using a specific handle | ||
# ManageIQ::Automate::System::CommonMethods::Utils::LogObject.log_and_notify(:level, msg, subject, handle) | ||
# | ||
def self.log_and_notify(level, message, subject, handle = $evm) |
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.
@billfitzgerald0120 We need to decide if the level passed into this method is a notify level or a log_level.
When this function is called will the user pass in level to be one of these ("info","error","warn","debug") or would it pass in the notify level ("success","error","warning","info") We have to pick one.
I was thinking it would be the notify level and then we would map from the notify_level to the log_level
so when we call
def self.log_and_notify(notify_level, message, subject, handle = $evm)
raise "Invalid notify level #{notify_level}" unless NOTIFY_LEVEL_TO_LOG_LEVEL.keys.include?(notify_level.downcase)
handle.create_notification(:level => notify_level.downcase, :message => message, :subject => subject)
handle.log(NOTIFY_LOG_TO_LOG_LEVEL(notify_level.downcase), message)
end
b7feba6
to
37e6d06
Compare
@@ -18,6 +25,20 @@ def self.log_and_exit(msg, exit_code, handle = $evm) | |||
handle.log('info', "Script ending #{msg} code : #{exit_code}") | |||
exit(exit_code) | |||
end | |||
|
|||
# If you want to create a notification and log a message without specifying a handle using the global $evm | |||
# ManageIQ::Automate::System::CommonMethods::Utils::LogObject.log_and_notify(:level, msg, subject) |
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.
@billfitzgerald0120 Do you want to change the comment to say :notify_level instead of level
# ManageIQ::Automate::System::CommonMethods::Utils::LogObject.log_and_notify(:level, msg, subject) | ||
# | ||
# If you want to create a notification and log a message using a specific handle | ||
# ManageIQ::Automate::System::CommonMethods::Utils::LogObject.log_and_notify(:level, msg, subject, handle) |
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.
@billfitzgerald0120 Also here in the command the level should be notify level
37e6d06
to
2afe93b
Compare
@mkanoor Changed comments as requested, please review |
This utility method allows an user to create a notification and log message with a single method call. You can create an info, warning, success or error notification. You can create an info, warn or error log message. @miq-bot add_label enhancement Made changes as requested. Changed comment as requested. Updated comment.
2afe93b
to
60959c6
Compare
Checked commit billfitzgerald0120@60959c6 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
For QE to test this feature
|
@billfitzgerald0120 @mkanoor |
@ganeshhubale No, as an enhancement these changes normally would not be back-ported. This is available in the |
This utility method allows an user to create a notification and log message with a single method call.
You can create an info, warning, success or error notification.
You can create an info, warn or error log message.
@miq-bot add_label enhancement