Valkyrie publish file.characterized event for derivative creation #6180
Conversation
| CreateDerivativesJob | ||
| .perform_later(file_set, event[:file_id], event[:path_hint]) | ||
| else | ||
| # With valkyrie, file.characterized event is also published for derivatives. |
There was a problem hiding this comment.
i think we don't want this to be true!
the valkyrie code path shouldn't be calling characterization on files that don't require characterization, right? i think this filter should go below around https://github.com/samvera/hyrax/pull/6180/files#diff-c1500de3cd457700a4445ea1b341164b5a822aa64a086d85e12eb9adfdfeba79R54, or else in the ValkyrieCharacterizationService?
There was a problem hiding this comment.
@no-reply Okay, I just moved the filter down to line https://github.com/samvera/hyrax/pull/6180/files#diff-c1500de3cd457700a4445ea1b341164b5a822aa64a086d85e12eb9adfdfeba79R54 as suggested. This will prevent it from running the file characterization job for derivative.s. But in UCSD dams, currently we have file characterization for all files, including derivatives.
There was a problem hiding this comment.
But in UCSD dams, currently we have file characterization for all files, including derivatives.
that seems okay. if downstream applications want to characterize under different conditions, it's easy to subscribe a new listener with the desired behavior.
51582a4 to
c42d0f8
Compare
| @@ -39,7 +47,9 @@ def on_file_metadata_updated(event) | |||
| # @param [Dry::Events::Event] event | |||
| # @return [void] | |||
| def on_file_uploaded(event) | |||
| # Run characterization | |||
| # Run characterization for original file only | |||
| return unless event[:metadata].original_file? | |||
There was a problem hiding this comment.
| return unless event[:metadata].original_file? | |
| return unless event[:metadata]&.original_file? |
:metadata probably SHOULD always be provided, but unhandled exceptions in listeners are very disruptive, so it's nice not to allow for nil errors.
There was a problem hiding this comment.
Done! Yep, that's what I thought. And the origin code event[:metadata].file in https://github.com/samvera/hyrax/pull/6180/files#diff-c1500de3cd457700a4445ea1b341164b5a822aa64a086d85e12eb9adfdfeba79L45 hinted me to remove that & sign 😄 .
c42d0f8 to
82a1166
Compare
82a1166 to
4ad22af
Compare
Fixes
Fixes #6162 ; refs #6162
Summary
Valkyrie publish file.characterized event for derivative creation.
@samvera/hyrax-code-reviewers