Skip to content
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

Batch Ingest Manifest with Pre-transcoded Derivatives Fails #4970

Open
joncameron opened this issue Nov 18, 2022 · 3 comments
Open

Batch Ingest Manifest with Pre-transcoded Derivatives Fails #4970

joncameron opened this issue Nov 18, 2022 · 3 comments
Assignees

Comments

@joncameron
Copy link
Contributor

Demo batch ingest manfiest rows with pre-transcoded derivatives fail with minio dropbox:

The following rows of your spreadsheet failed:
Row	Error
4 	undefined method `close' for #<Addressable::URI:0x0000562fc2cc0bf8> Did you mean? clone
5 	undefined method `close' for #<Addressable::URI:0x00007f236d479158> Did you mean? clone 
@cjcolvar cjcolvar self-assigned this Nov 18, 2022
@cjcolvar
Copy link
Member

I think that this scenario has never worked. Handling of pre-transcoded derivatives does not work with s3 sources and would require a medium amount of work (3 to 5). Is this a use case we need to support or can it at least be added to known issues (or unuspported configurations) for now?

@cjcolvar
Copy link
Member

For future reference, this is where I started working on adding support for s3 hosted pre-transcoded derivatives:

diff --git a/app/models/master_file.rb b/app/models/master_file.rb
index fa4ad0bb..c87511bd 100644
--- a/app/models/master_file.rb
+++ b/app/models/master_file.rb
@@ -695,17 +695,31 @@ class MasterFile < ActiveFedora::Base
   end
 
   def saveDerivativesHash(derivative_hash)
-    usable_files = derivative_hash.select { |quality, file| File.file?(file) }
-    self.working_file_path = usable_files.values.collect { |file| create_working_file!(File.realpath(file)) }.compact
+    if derivative_hash.values.all? { |file| (file.is_a?(URI) || file.is_a?(Addressable::URI)) && file.scheme == "s3" }
+      self.working_file_path = derivative_hash.values
+      %w(quality-low quality-medium quality-high).each do |quality|
+        next unless usable_files.has_key?(quality)
+       self.file_location = file.to_s
+        self.file_size = FileLocator::S3File.new(file).object.size
+        break
+      end
 
-    %w(quality-high quality-medium quality-low).each do |quality|
-      next unless usable_files.has_key?(quality)
-      self.file_location = File.realpath(usable_files[quality])
-      self.file_size = usable_files[quality].size.to_s
-      break
+      return
+    end
+
+    begin
+      usable_files = derivative_hash.select { |quality, file| File.file?(file) }
+      self.working_file_path = usable_files.values.collect { |file| create_working_file!(File.realpath(file)) }.compact
+
+      %w(quality-high quality-medium quality-low).each do |quality|
+        next unless usable_files.has_key?(quality)
+        self.file_location = File.realpath(usable_files[quality])
+        self.file_size = usable_files[quality].size.to_s
+        break
+      end
+    ensure
+      derivative_hash.values.map { |file| file.close }
     end
-  ensure
-    derivative_hash.values.map { |file| file.close }
   end
 
   def reloadTechnicalMetadata!

@elynema
Copy link
Contributor

elynema commented Nov 28, 2022

Note this as a known issue and move to backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants