From 1c1ed0dd9d3a7d34545f44fe555ba884fac938c1 Mon Sep 17 00:00:00 2001 From: Justin Edmund Date: Sat, 18 Jan 2025 11:46:41 -0800 Subject: [PATCH] Work towards fixing embed images (#174) * Add Redis and Sidekiq * Rename PreviewGenerationMonitor * Update production.rb require master key * Initialize AWS at application start * Add fallbacks for credentials * Add logging * Create railway.toml --- Gemfile | 6 ++ Gemfile.lock | 11 +++ app/jobs/generate_party_preview_job.rb | 10 +- app/services/aws_service.rb | 91 ++++++++++++------- app/services/preview_service/canvas.rb | 22 ++++- app/services/preview_service/coordinator.rb | 44 +++++++-- ...ation_monitor.rb => generation_monitor.rb} | 5 +- config/environments/production.rb | 3 +- config/initializers/aws.rb | 10 ++ config/initializers/redis.rb | 9 ++ config/initializers/scheduler.rb | 6 +- config/initializers/sidekiq.rb | 14 +++ railway.toml | 16 ++++ 13 files changed, 191 insertions(+), 56 deletions(-) rename app/services/preview_service/{preview_generation_monitor.rb => generation_monitor.rb} (95%) create mode 100644 config/initializers/aws.rb create mode 100644 config/initializers/redis.rb create mode 100644 config/initializers/sidekiq.rb create mode 100644 railway.toml diff --git a/Gemfile b/Gemfile index 692fac3..42f9853 100644 --- a/Gemfile +++ b/Gemfile @@ -47,6 +47,12 @@ gem 'email_validator' # pg_search builds ActiveRecord named scopes that take advantage of PostgreSQL’s full text search gem 'pg_search' +# A Ruby client library for Redis +gem 'redis' + +# Simple, efficient background processing for Ruby +gem 'sidekiq' + # scheduler for Ruby (at, in, cron and every jobs) gem 'rufus-scheduler' diff --git a/Gemfile.lock b/Gemfile.lock index a4b50aa..bac6941 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -299,6 +299,10 @@ GEM rbs (2.8.4) rdoc (6.10.0) psych (>= 4.0.0) + redis (5.3.0) + redis-client (>= 0.22.0) + redis-client (0.23.2) + connection_pool regexp_parser (2.10.0) reline (0.6.0) io-console (~> 0.5) @@ -351,6 +355,11 @@ GEM securerandom (0.4.1) shoulda-matchers (6.4.0) activesupport (>= 5.2.0) + sidekiq (7.3.7) + connection_pool (>= 2.3.0) + logger + rack (>= 2.2.4) + redis-client (>= 0.22.2) simplecov (0.22.0) docile (~> 1.1) simplecov-html (~> 0.11) @@ -444,6 +453,7 @@ DEPENDENCIES puma rack-cors rails + redis responders rspec-rails rspec_junit_formatter @@ -451,6 +461,7 @@ DEPENDENCIES rufus-scheduler sdoc shoulda-matchers + sidekiq simplecov solargraph spring diff --git a/app/jobs/generate_party_preview_job.rb b/app/jobs/generate_party_preview_job.rb index 2d87f9b..b73a7f4 100644 --- a/app/jobs/generate_party_preview_job.rb +++ b/app/jobs/generate_party_preview_job.rb @@ -12,9 +12,7 @@ class GeneratePartyPreviewJob < ApplicationJob around_perform :track_timing def perform(party_id) - # Log start of job processing Rails.logger.info("Starting preview generation for party #{party_id}") - party = Party.find(party_id) if party.preview_state == 'generated' && @@ -26,7 +24,10 @@ class GeneratePartyPreviewJob < ApplicationJob begin service = PreviewService::Coordinator.new(party) + Rails.logger.info("Created PreviewService::Coordinator") + result = service.generate_preview + Rails.logger.info("Generate preview result: #{result}") if result Rails.logger.info("Successfully generated preview for party #{party_id}") @@ -36,9 +37,10 @@ class GeneratePartyPreviewJob < ApplicationJob end rescue => e Rails.logger.error("Error generating preview for party #{party_id}: #{e.message}") - Rails.logger.error(e.backtrace.join("\n")) + Rails.logger.error("Full error details:") + Rails.logger.error(e.full_message) # This will include the stack trace notify_failure(party, e) - raise # Allow retry mechanism to handle the error + raise end end diff --git a/app/services/aws_service.rb b/app/services/aws_service.rb index 3fa5997..4fc0f93 100644 --- a/app/services/aws_service.rb +++ b/app/services/aws_service.rb @@ -1,21 +1,21 @@ -# frozen_string_literal: true - -require 'aws-sdk-s3' - class AwsService attr_reader :s3_client, :bucket class ConfigurationError < StandardError; end def initialize - validate_credentials! + Rails.logger.info "Environment: #{Rails.env}" + + # Try different methods of getting credentials + creds = get_credentials + Rails.logger.info "Credentials source: #{creds[:source]}" @s3_client = Aws::S3::Client.new( - region: Rails.application.credentials.dig(:aws, :region), - access_key_id: Rails.application.credentials.dig(:aws, :access_key_id), - secret_access_key: Rails.application.credentials.dig(:aws, :secret_access_key) + region: creds[:region], + access_key_id: creds[:access_key_id], + secret_access_key: creds[:secret_access_key] ) - @bucket = Rails.application.credentials.dig(:aws, :bucket_name) + @bucket = creds[:bucket_name] rescue KeyError => e raise ConfigurationError, "Missing AWS credential: #{e.message}" end @@ -40,30 +40,59 @@ class AwsService private - def credentials - @credentials ||= begin - creds = Rails.application.credentials[:aws] - raise ConfigurationError, 'AWS credentials not found' unless creds - - { - region: creds[:region], - access_key_id: creds[:access_key_id], - secret_access_key: creds[:secret_access_key], - bucket_name: creds[:bucket_name] - } - end - end - - def validate_credentials! - missing = [] - creds = Rails.application.credentials[:aws] - - %i[region access_key_id secret_access_key bucket_name].each do |key| - missing << key unless creds&.dig(key) + def get_credentials + # Try Rails credentials first + rails_creds = Rails.application.credentials.dig(:aws) + if rails_creds&.dig(:access_key_id) + Rails.logger.info "Using Rails credentials" + return rails_creds.merge(source: 'rails_credentials') end - return unless missing.any? + # Try string keys + rails_creds = Rails.application.credentials.dig('aws') + if rails_creds&.dig('access_key_id') + Rails.logger.info "Using Rails credentials (string keys)" + return { + region: rails_creds['region'], + access_key_id: rails_creds['access_key_id'], + secret_access_key: rails_creds['secret_access_key'], + bucket_name: rails_creds['bucket_name'], + source: 'rails_credentials_string' + } + end - raise ConfigurationError, "Missing AWS credentials: #{missing.join(', ')}" + # Try environment variables + if ENV['AWS_ACCESS_KEY_ID'] + Rails.logger.info "Using environment variables" + return { + region: ENV['AWS_REGION'], + access_key_id: ENV['AWS_ACCESS_KEY_ID'], + secret_access_key: ENV['AWS_SECRET_ACCESS_KEY'], + bucket_name: ENV['AWS_BUCKET_NAME'], + source: 'environment' + } + end + + # Try alternate environment variable names + if ENV['RAILS_AWS_ACCESS_KEY_ID'] + Rails.logger.info "Using Rails-prefixed environment variables" + return { + region: ENV['RAILS_AWS_REGION'], + access_key_id: ENV['RAILS_AWS_ACCESS_KEY_ID'], + secret_access_key: ENV['RAILS_AWS_SECRET_ACCESS_KEY'], + bucket_name: ENV['RAILS_AWS_BUCKET_NAME'], + source: 'rails_environment' + } + end + + validate_credentials = ->(creds, source) { + missing = [] + %i[region access_key_id secret_access_key bucket_name].each do |key| + missing << key unless creds[key].present? + end + raise ConfigurationError, "Missing AWS credentials from #{source}: #{missing.join(', ')}" if missing.any? + } + + raise ConfigurationError, "No AWS credentials found in any location" end end diff --git a/app/services/preview_service/canvas.rb b/app/services/preview_service/canvas.rb index 95879fb..1477ec0 100644 --- a/app/services/preview_service/canvas.rb +++ b/app/services/preview_service/canvas.rb @@ -15,12 +15,24 @@ module PreviewService end def create_blank_canvas(width: PREVIEW_WIDTH, height: PREVIEW_HEIGHT, color: DEFAULT_BACKGROUND_COLOR) - temp_file = Tempfile.new(%w[canvas .png]) + Rails.logger.info("Checking ImageMagick installation...") + version = `convert -version` + Rails.logger.info("ImageMagick version: #{version}") - MiniMagick::Tool::Convert.new do |convert| - convert.size "#{width}x#{height}" - convert << "xc:#{color}" - convert << temp_file.path + temp_file = Tempfile.new(%w[canvas .png]) + Rails.logger.info("Created temp file: #{temp_file.path}") + + begin + MiniMagick::Tool::Convert.new do |convert| + convert.size "#{width}x#{height}" + convert << "xc:#{color}" + convert << temp_file.path + end + Rails.logger.info("Canvas created successfully") + rescue => e + Rails.logger.error("Failed to create canvas: #{e.message}") + Rails.logger.error(e.backtrace.join("\n")) + raise end temp_file diff --git a/app/services/preview_service/coordinator.rb b/app/services/preview_service/coordinator.rb index b6fb964..2fad8a6 100644 --- a/app/services/preview_service/coordinator.rb +++ b/app/services/preview_service/coordinator.rb @@ -41,22 +41,33 @@ module PreviewService Rails.logger.info("Starting preview generation for party #{@party.id}") set_generation_in_progress - # Generate the preview image + Rails.logger.info("Creating preview image...") image = create_preview_image - save_preview(image) + Rails.logger.info("Preview image created successfully") - # Update party state + Rails.logger.info("Saving preview...") + save_preview(image) + Rails.logger.info("Preview saved successfully") + + Rails.logger.info("Updating party state...") @party.update!( preview_state: :generated, preview_generated_at: Time.current ) + Rails.logger.info("Party state updated successfully") + true rescue => e + Rails.logger.error("Failed to generate preview: #{e.class} - #{e.message}") + Rails.logger.error("Stack trace:") + Rails.logger.error(e.backtrace.join("\n")) handle_preview_generation_error(e) false ensure + Rails.logger.info("Cleaning up resources...") @image_fetcher.cleanup clear_generation_in_progress + Rails.logger.info("Cleanup completed") end end @@ -99,28 +110,34 @@ module PreviewService # # @return [MiniMagick::Image] The generated preview image def create_preview_image - # Create blank canvas + Rails.logger.info("Creating blank canvas...") canvas = @canvas_service.create_blank_canvas image = MiniMagick::Image.new(canvas.path) + Rails.logger.info("Blank canvas created") - # Fetch job icon + Rails.logger.info("Processing job icon...") job_icon = nil if @party.job.present? + Rails.logger.info("Fetching job icon for job ID: #{@party.job.granblue_id}") job_icon = @image_fetcher.fetch_job_icon(@party.job.granblue_id) + Rails.logger.info("Job icon fetched successfully") if job_icon end - # Add party name with job icon + Rails.logger.info("Adding party name and job icon...") text_result = @canvas_service.add_text(image, @party.name, job_icon: job_icon, user: @party.user) image = text_result[:image] + Rails.logger.info("Party name and job icon added") - # Calculate grid layout + Rails.logger.info("Calculating grid layout...") grid_layout = @grid_service.calculate_layout( canvas_height: Canvas::PREVIEW_HEIGHT, title_bottom_y: text_result[:text_bottom_y] ) + Rails.logger.info("Grid layout calculated") - # Organize and draw weapons + Rails.logger.info("Drawing weapons...") image = organize_and_draw_weapons(image, grid_layout) + Rails.logger.info("Weapons drawn successfully") image end @@ -196,15 +213,22 @@ module PreviewService begin image.write(temp_file.path) + # Use timestamped filename similar to local storage + timestamp = Time.current.strftime('%Y%m%d%H%M%S') + key = "#{PREVIEW_FOLDER}/#{@party.shortcode}_#{timestamp}.png" + File.open(temp_file.path, 'rb') do |file| @aws_service.s3_client.put_object( - bucket: S3_BUCKET, - key: preview_key, + bucket: @aws_service.bucket, + key: key, body: file, content_type: 'image/png', acl: 'private' ) end + + # Optionally, store this key on the party record if needed for retrieval + @party.update!(preview_s3_key: key) ensure temp_file.close temp_file.unlink diff --git a/app/services/preview_service/preview_generation_monitor.rb b/app/services/preview_service/generation_monitor.rb similarity index 95% rename from app/services/preview_service/preview_generation_monitor.rb rename to app/services/preview_service/generation_monitor.rb index 74b7c37..98bc2ef 100644 --- a/app/services/preview_service/preview_generation_monitor.rb +++ b/app/services/preview_service/generation_monitor.rb @@ -1,6 +1,7 @@ -# app/services/preview_generation_monitor.rb +require 'sidekiq/api' + module PreviewService - class PreviewGenerationMonitor + class GenerationMonitor class << self def check_stalled_jobs Party.where(preview_state: :queued) diff --git a/config/environments/production.rb b/config/environments/production.rb index bf24f9a..bd8c39b 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -2,7 +2,8 @@ require "active_support/core_ext/integer/time" Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. - + config.require_master_key = true + # Code is not reloaded between requests. config.cache_classes = true diff --git a/config/initializers/aws.rb b/config/initializers/aws.rb new file mode 100644 index 0000000..62ff71b --- /dev/null +++ b/config/initializers/aws.rb @@ -0,0 +1,10 @@ +Rails.application.config.after_initialize do + Rails.logger.info "Initializing AWS Service..." + begin + AwsService.new + Rails.logger.info "AWS Service initialized successfully" + rescue => e + Rails.logger.error "Failed to initialize AWS Service: #{e.message}" + Rails.logger.error e.backtrace.join("\n") + end +end diff --git a/config/initializers/redis.rb b/config/initializers/redis.rb new file mode 100644 index 0000000..48c1bb4 --- /dev/null +++ b/config/initializers/redis.rb @@ -0,0 +1,9 @@ +# Fetch environment variables with defaults if not set +redis_url = ENV.fetch('REDIS_URL', 'redis://localhost') +redis_port = ENV.fetch('REDISPORT', '6379') + +# Combine URL and port (adjust the path/DB as needed) +full_redis_url = "#{redis_url}/0" + +# Initialize Redis using the constructed URL +$redis = Redis.new(url: full_redis_url) diff --git a/config/initializers/scheduler.rb b/config/initializers/scheduler.rb index ec3e92a..1ed6488 100644 --- a/config/initializers/scheduler.rb +++ b/config/initializers/scheduler.rb @@ -5,14 +5,14 @@ unless defined?(Rails::Console) || Rails.env.test? || File.split($0).last == 'ra scheduler = Rufus::Scheduler.new scheduler.every '5m' do - PreviewGenerationMonitor.check_stalled_jobs + PreviewService::GenerationMonitor.check_stalled_jobs end scheduler.every '1h' do - PreviewGenerationMonitor.retry_failed + PreviewService::GenerationMonitor.retry_failed end scheduler.every '1d' do - PreviewGenerationMonitor.cleanup_old_previews + PreviewService::GenerationMonitor.cleanup_old_previews end end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb new file mode 100644 index 0000000..48408f7 --- /dev/null +++ b/config/initializers/sidekiq.rb @@ -0,0 +1,14 @@ +# Fetch environment variables with defaults if not set +redis_url = ENV.fetch('REDIS_URL', 'redis://localhost') +redis_port = ENV.fetch('REDISPORT', '6379') + +# Combine URL and port (adjust the path/DB as needed) +full_redis_url = "#{redis_url}/0" + +Sidekiq.configure_server do |config| + config.redis = { url: full_redis_url } +end + +Sidekiq.configure_client do |config| + config.redis = { url: full_redis_url } +end diff --git a/railway.toml b/railway.toml new file mode 100644 index 0000000..2cdf95e --- /dev/null +++ b/railway.toml @@ -0,0 +1,16 @@ +[phases.setup] +nixPkgs = [ + "imagemagick", + "ghostscript", # For PDF/PS operations + "pkgconfig", # For gem compilation + "libmagickwand" # ImageMagick C library +] + +[phases.install] +dependsOn = ["setup"] + +[phases.build] +dependsOn = ["install"] + +[start] +cmd = "bin/rails server"