-
Notifications
You must be signed in to change notification settings - Fork 6
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
Dist-10392 add logging functionality #55
Conversation
1) add muffin logger
1) add request and response logging for finance API call
1) fix call to logger
1) rubocop fixes
lib/muffin_man/finances/v0.rb
Outdated
@@ -15,7 +16,10 @@ def list_financial_event_groups(max_results_per_page = nil, financial_event_grou | |||
end | |||
@query_params["NextToken"] = next_token unless next_token.nil? | |||
@request_type = "GET" | |||
call_api | |||
res = call_api |
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.
can we add this in call_api so that any api calls can use this and add enableLogging module wise?
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.
done
1) add log enabler
1) move log_request_and_response to enable_logger
1) fix rubocop offence
lib/muffin_man/muffin_logger.rb
Outdated
module MuffinMan | ||
class MuffinLogger | ||
def self.logger | ||
defined?(Rails) ? Rails.logger : Logger.new($stdout) |
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.
Instead of making this gem aware of rails, we can add support for the application using the gem to set a logger. Like how Rails supports doing Rails.logger = Logger.new
. To do this you can add :logger
as an attr_accessor
like we have done for :configuration
on this line
class << self
attr_accessor :configuration, :logger
end
Then we can use the logger set by the application if one is set with
MuffinMan.logger.info "a message for the logs"
If no logger is set, I wonder if we default to Logger.new("/dev/null")
so that nothing actually gets logged but we can still write code assuming MuffinMan.logger
is always set.
443aa70
to
6793d70
Compare
6793d70
to
90e6e98
Compare
28023f5
to
92170cb
Compare
28cbd29
to
7898564
Compare
7898564
to
8aa2fb5
Compare
* Reorganize MuffinMan logging setup * Support using Typheous verbose mode with ENV variable
Add logging in muffin gem. Use Rails logger if the gem is used in rails app or use default ruby logger
Allow enabling of error and info logging Amazon request and response