Add sane defaults for logfile names

Gemfile.lock

@@ -20,6 +20,7 @@ GEM
       faraday-net_http (>= 2.0, < 3.1)
       ruby2_keywords (>= 0.0.4)
     faraday-net_http (3.0.2)
+    ffi (1.15.5)
     ffi (1.15.5-java)
     ffi-compiler (1.0.1)
       ffi (>= 1.0.0)
@@ -38,6 +39,7 @@ GEM
     httpx (0.23.3)
       http-2-next (>= 0.4.1)
     jdbc-mysql (8.0.27)
+    json (2.6.3)
     json (2.6.3-java)
     language_server-protocol (3.17.0.3)
     library_stdnums (1.6.0)
@@ -59,16 +61,22 @@ GEM
     match_map (3.0.0)
     method_source (1.0.0)
     naconormalizer (1.0.1-java)
+    nokogiri (1.15.2-arm64-darwin)
+      racc (~> 1.4)
     nokogiri (1.15.2-java)
       racc (~> 1.4)
     parallel (1.23.0)
     parser (3.2.2.1)
       ast (~> 2.4.1)
+    pry (0.14.2)
+      coderay (~> 1.1)
+      method_source (~> 1.0)
     pry (0.14.2-java)
       coderay (~> 1.1)
       method_source (~> 1.0)
       spoon (~> 0.0)
     public_suffix (5.0.1)
+    racc (1.6.2)
     racc (1.6.2-java)
     rainbow (3.1.1)
     rake (13.0.6)
@@ -152,12 +160,16 @@ GEM
       marc_alephsequential (~> 2)
       yell
     traject_umich_format (0.4.4)
+    unf (0.1.4)
+      unf_ext
     unf (0.1.4-java)
+    unf_ext (0.0.8.2)
     unicode-display_width (2.4.2)
     yell (2.2.2)
     zinzout (0.1.1)
 
 PLATFORMS
+  arm64-darwin-22
   universal-java-17
 
 DEPENDENCIES
@@ -191,4 +203,4 @@ DEPENDENCIES
   zinzout
 
 BUNDLED WITH
-   2.4.13
+   2.4.17

lib/cictl/base_command.rb

@@ -2,28 +2,41 @@
 
 require "thor"
 require_relative "common"
+require_relative "../services"
 
 module CICTL
   class BaseCommand < Thor
     include Common
 
     class_option :verbose, type: :boolean,
-      desc: "Emit 'debug' in addition to 'info' log entries"
+      desc: "Emit 'debug' in addition to 'info' log entries",
+      default: false
     class_option :log, type: :string,
-      desc: "Log to <logfile> instead of STDOUT/STDERR",
+      desc: "Log to <logfile> in <logdir>. Use 'daily' or 'full' for sane defaults.",
       banner: "<logfile>"
+    class_option :logdir, type: :string,
+      desc: "Location for default logs",
+      default: HathiTrust::Services[:logfile_directory]
+    class_option :quiet, type: :boolean,
+      desc: "Suppress normal output to STDERR",
+      default: false
 
     def initialize(args = [], local_options = {}, config = {})
       # For creating the default CICTL logger as well as one for calling Traject
       # an any other subcomponents we want to stick a custom logger into.
+      super args, local_options, config
+      # if @options[:logdir]
+      #   LogfileDefaults.logdir = @options[:logdir]
+      # end
+
       HathiTrust::Services.register(:logger_factory) do
-        LoggerFactory.new(verbose: options[:verbose], log_file: options[:log])
+        LoggerFactory.new(verbose: options[:verbose], log_file: options[:log], quiet: options[:quiet])
       end
+
       # Default CICTL logger
       HathiTrust::Services.register(:logger) do
         HathiTrust::Services[:logger_factory].logger
       end
-      super args, local_options, config
     end
   end
 end

lib/cictl/logfile_defaults.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require_relative "../services"
+require "pathname"
+require "date"
+require "date_named_file"
+
+module CICTL
+  module LogfileDefaults
+    extend self
+
+    def logdir
+      HathiTrust::Services[:logfile_directory]
+    end
+
+    def logdir=(path)
+      Pathname.new(path).mkpath
+      HathiTrust::Services.register(:logfile_directory) { path }
+    end
+
+    def filepath_of(str)
+      (Pathname.new(logdir) + str).to_s
+    end
+
+    def today_yyyymmdd
+      Date.today.strftime("%Y%m%d")
+    end
+
+    alias_method :daily_yyyymmdd, :today_yyyymmdd
+
+    def daily_template
+      DateNamedFile.new("daily_%Y%m%d.log").in_dir(logdir)
+    end
+
+    def full_template
+      DateNamedFile.new("full_%Y%m%d.log").in_dir(logdir)
+    end
+
+    def daily(yyyymmdd = today_yyyymmdd)
+      daily_template.at(yyyymmdd)
+    end
+
+    alias_method :today, :daily
+
+    def full(yyyymmdd = today_yyyymmdd)
+      full_template.at(yyyymmdd)
+    end
+  end
+end

lib/cictl/logger_factory.rb

@@ -1,25 +1,52 @@
 # frozen_string_literal: true
 
 require "semantic_logger"
+require_relative "logfile_defaults"
 
 module CICTL
   class LoggerFactory
-    def initialize(verbose: false, log_file: nil)
+    class Formatter < SemanticLogger::Formatters::Default
+      # Return the complete log level name in uppercase instead of one letter
+      def level
+        log.level.upcase
+      end
+    end
+
+    def initialize(verbose: false, log_file: nil, quiet: false)
       @verbose = verbose
       @log_file = log_file
+      @quiet = quiet
     end
 
     def logger(owner: "CICTL")
+      # Force SemanticLogger to run in main thread. This is only for testing.
+      # The alternative -- logger.close -- makes the GitHub testing environment very unhappy.
+      SemanticLogger.sync! if ENV["CICTL_SEMANTIC_LOGGER_SYNC"]
       # If we use more than one factory (as happens in the tests but not yet in the main code) we get warnings
       # "Ignoring attempt to add a second console appender: … since it would result in duplicate console output."
       # Lazily apply SemanticLogger config setup here instead of the initializer
       # to make sure no one changes the appenders before the logger is created.
-      SemanticLogger.default_level = min_level
       SemanticLogger.clear_appenders!
-      if @log_file
-        SemanticLogger.add_appender(file_name: @log_file, level: min_level)
+      SemanticLogger.default_level = min_level
+
+      Pathname.new(LogfileDefaults.logdir).mkpath
+
+      logfile_path = case @log_file
+      when "daily", "today"
+        LogfileDefaults.daily
+      when "full"
+        LogfileDefaults.full
+      when String
+        LogfileDefaults.logdir + "/#{@log_file}"
+      end
+
+      if logfile_path
+        SemanticLogger.add_appender(file_name: logfile_path.to_s, level: min_level, formatter: Formatter.new)
       end
-      SemanticLogger.add_appender(io: $stderr, level: :error)
+      unless @quiet
+        SemanticLogger.add_appender(io: $stderr, level: :error, formatter: Formatter.new)
+      end
+
       SemanticLogger[owner]
     end
 

lib/services.rb

@@ -52,6 +52,11 @@ def env_local_file
     ENV["DDIR"] || "/htsolr/catalog/prep"
   end
 
+  Services.register(:logfile_directory) do
+    default = "#{HOME}/logs"
+    ENV["LOG_DIR"] || default
+  end
+
   Services.register(:db) do
     DBH.connect
   end

spec/cictl/delete_command_spec.rb

@@ -11,6 +11,7 @@
   after(:each) do
     CICTL::SolrClient.new.empty!.commit!
     ENV.delete "CICTL_ZEPHIR_FILE_TEMPLATE_PREFIX"
+    remove_test_log
   end
 
   describe "#delete all" do

spec/cictl/index_command_spec.rb

@@ -12,14 +12,15 @@
   after(:each) do
     CICTL::SolrClient.new.empty!.commit!
     ENV.delete "CICTL_ZEPHIR_FILE_TEMPLATE_PREFIX"
+    remove_test_log
   end
 
   describe "#index all" do
     it "indexes all example records" do
       # Make a fake delete entry for a bogus id
       bogus_delete = "000000000"
       CICTL::SolrClient.new.set_deleted [bogus_delete]
-      CICTL::Commands.start(["index", "all", "--no-wait", "--log", test_log])
+      CICTL::Commands.start(["index", "all", "--no-wait", "--quiet", "--log", test_log])
       expect(solr_count).to eq CICTL::Examples.all_ids.count + 1
       expect(solr_deleted_count).to be > 0
       expect(solr_ids("deleted:true")).to include(bogus_delete)

spec/cictl/logger_factory_spec.rb

@@ -1,32 +1,47 @@
 # frozen_string_literal: true
 
 require "spec_helper"
+require_relative "../../lib/cictl/logfile_defaults"
+require_relative "../../lib/services"
 
-RSpec.describe CICTL::LoggerFactory, skip: true do
-  shared_examples "any logger" do |verbose, log_file|
-    subject { described_class.new(verbose: verbose, log_file: log_file).logger }
-    it "sends #error to STDERR" do
-      expect { subject.error "error shwoozle" }.to output(/shwoozle/).to_stderr
-    end
+ENV["CICTL_SEMANTIC_LOGGER_SYNC"] = "1"
+RSpec.describe CICTL::LoggerFactory do
+  def testlogger(verbose: false, log_file: test_log, quiet: false)
+    CICTL::LoggerFactory.new(verbose: verbose, log_file: log_file, quiet: quiet).logger
+  end
 
-    it "sends #fatal to STDERR" do
-      expect { subject.fatal "fatal shwoozle" }.to output(/shwoozle/).to_stderr
-    end
+  after(:each) do
+    remove_test_log
+  end
 
-    it "does not send anything less than #error to STDERR" do
-      %i[debug info warn].each do |level|
-        expect { subject.send(level, "#{level} shwoozle") }.not_to output(/shwoozle/).to_stderr
-      end
-    end
+  it "sends #error to $stderr" do
+    expect {
+      testlogger.error "error error-via-error"
+    }.to output(/error-via-error/).to_stderr_from_any_process
   end
 
-  describe "#logger" do
-    context "with a log file" do
-      it_behaves_like "any logger", false, test_log
-    end
+  it "sends #fatal to $stderr" do
+    expect {
+      testlogger.fatal "fatal shwoozle"
+    }.to output(/shwoozle/).to_stderr_from_any_process
+  end
+
+  it "sends stuff to the logfile" do
+    testlogger.error "error-in-file"
+    expect(File.read(HathiTrust::Services[:logfile_directory] + "/" + test_log)).to match(/error-in-file/)
+  end
 
-    context "without a log file" do
-      it_behaves_like "any logger", false, nil
+  it "does not send anything less than #error to STDERR" do
+    %i[debug info warn].each do |level|
+      expect {
+        testlogger.send(level, "#{level} shwoozle")
+      }.not_to output(/shwoozle/).to_stderr_from_any_process
     end
   end
+
+  it "doesn't send output to stderr in quiet mode" do
+    expect {
+      testlogger(quiet: true).error("Error")
+    }.not_to output(/Error/).to_stderr_from_any_process
+  end
 end

spec/spec_helper.rb

@@ -119,6 +119,12 @@ def test_log
   "TEST_LOG.txt"
 end
 
+def remove_test_log
+  FileUtils.rm(CICTL::LogfileDefaults.filepath_of(test_log))
+rescue Errno::ENOENT
+  # file wasn't there, and that's fine
+end
+
 def solr_count
   CICTL::SolrClient.new.count
 end