SciRuby / daru-io

daru-io is a plugin gem to the existing daru gem, which aims to add support to Importing DataFrames from / Exporting DataFrames to multiple formats.
http://www.rubydoc.info/github/athityakumar/daru-io/master/
MIT License
24 stars 9 forks source link

add importer for log files #75

Closed rohitner closed 6 years ago

rohitner commented 6 years ago

This PR integrates the gem request-log-analyzer to convert rails logs to Daru::DataFrame.

rohitner commented 6 years ago

Please review the changes so that I can start the documentation and fix rubocop tests.

rohitner commented 6 years ago

@zverok @athityakumar please review and suggest changes for rubocop failure.

rohitner commented 6 years ago

How about adding importers of all three formats in a single file? It would eliminate the additional separate module for the patch. The log.rb file will look like this

module Importers
  class Log < Base
    Daru::Dataframe.register_io_module :read_log, self
    ORDER_RAILS  = %i[...].freeze
    ORDER_APACHE = %i[...].freeze
    ORDER_S3     = %i[...].freeze
    ...
    def read(path, format: :rails3) # default format is :rails3
      case format
      when :rails3    then order = ORDER_RAILS
      when :apache    then order = ORDER_APACHE
      when :amazon_s3 then order = ORDER_S3
      else raise ArgumentError, 'Invalid format provided.'
      ...
      @file_data = parser.parse_hash(path, order)
      self
    end
    ...
    private
    module RequestLogAnalyzerPatch
    ...
      def parse_hash(file, order)
      ...
          parsed_list[i] = order.map ...
      ...
      end
    end
  end
end

In the above method, I will have need to modify the line containing register_io_module to take one more argument specifying the format. WDYT?

zverok commented 6 years ago

How about adding importers of all three formats in a single file? It would eliminate the additional separate module for the patch. The log.rb file will look like this

That may work. Just the code can be simpler:

ORDERS = {
  rails: %i[...].freeze,
  apache: %i[...].freeze,
  s3: %i[...].freeze
}
# and then...
order = ORDERS.fetch(format)
rohitner commented 6 years ago

@zverok please review

zverok commented 6 years ago

(Just turn off the Lint/SplatKeywordArguments, it is honestly meaningless and already removed in the master).

zverok commented 6 years ago

Merged! Congrats :tada: