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

Adding Samvera::Derivatives interface and Hyrax shim #761

Closed
wants to merge 4 commits into from

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Mar 16, 2023

Adding Samvera::Derivatives for interface discussion

53e80f5

This commit begins to define an interface for the separation of concerns
regarding the long-standing implementation of Hydra::Derivatives and
Hyrax::FileSetDerivativesService as those relate to the
existence of pre-existing derivatives.

This does not address the implementation details related to Bulkrax
handling this work. I would instead assume that Bulkrax would implement
a subclass(es) Samvera::Derivatives::FileLocator::Strategy, which
would in turn address the specific needs and implementation details.

As I've explored the implementations, a challenge of Hydra::Derivatives
is that is combines the concern of finding the derivative (by making it)
and applying that derivative to the corresponding file set.

Also, there is a higher level concern that is not addressed in this
implementation; namely what are the derivatives of concern for a
given FileSet; something that is likely based on the mime_type of the
original file and metadata from the parent work.

Why not create another gem?

  1. The immediate need is Bulkrax related.
  2. The plumbing is laid for Bulkrax's test suite.
  3. We probably don't need another gem to triangulate for a general
    solution.

That said, I am name spacing this interface in Samvera so if we find
it useful we can extract a samvera-derivatives gem. After all, not
everyone's going to want Bulkrax.

Why not put this in Hyrax?

  1. Because Bulkrax works across many different Hyrax versions. Thus by
    being in Bulkrax it is generally more available than if it were in
    Hyrax; something which we'd need to backport.

An advantage of establishing the interface is that work can be done to
now "fan out". That is someone can work on a locator strategy while
another works on the applicator strategy.

Related to:

Introducing location concept to Samvera::Derivatives

42df4df

Prior to this commit, I was passing an array of strings back from the
locator. With that pathway, I was ignoring how the existing
Hyrax::FileSetDerivativesService would likely be used as a fallback.

With this commit, I'm beginning to account for how that interaction
would be; in part by focusing on a known use case and building the
interactions accordingly.

This commit stencils in the expected behavior in what I think is
adequate to convey intention and design.

Related to:

Adding Samvera::Derivatives::Hyrax consideration

e33e454

With this commit, I begin demonstrating how the original configuration
can be leveraged to do the derivative work. This still assumes a
configuration for derivatives; something that I continue to punt on
until I have more concrete code that I can further build from.

Related to:

This commit begins to define an interface for the separation of concerns
regarding the long-standing implementation of `Hydra::Derivatives` and
`Hyrax::FileSetDerivativesService` as those relate to the
existence of pre-existing derivatives.

This does not address the implementation details related to Bulkrax
handling this work.  I would instead assume that Bulkrax would implement
a subclass(es) `Samvera::Derivatives::FileLocator::Strategy`, which
would in turn address the specific needs and implementation details.

As I've explored the implementations, a challenge of Hydra::Derivatives
is that is combines the concern of finding the derivative (by making it)
and applying that derivative to the corresponding file set.

Also, there is a higher level concern that is not addressed in this
implementation; namely what are the derivatives of concern for a
given `FileSet`; something that is likely based on the mime_type of the
original file and metadata from the parent work.

Why not create another gem?

1. The immediate need is Bulkrax related.
2. The plumbing is laid for Bulkrax's test suite.
3. We probably don't need another gem to triangulate for a general
   solution.

That said, I am name spacing this interface in `Samvera` so if we find
it useful we can extract a `samvera-derivatives` gem.  After all, not
everyone's going to want Bulkrax.

Why not put this in Hyrax?

1. Because Bulkrax works across many different Hyrax versions.  Thus by
   being in Bulkrax it is generally more available than if it were in
   Hyrax; something which we'd need to backport.

An advantage of establishing the interface is that work can be done to
now "fan out".  That is someone can work on a locator strategy while
another works on the applicator strategy.

Related to:

- #760
- notch8/utk-hyku#343
# that will create the thumbnail and write it to the location. The two applicator
# strategies in that case would be the wrapper and logic that will write the found
# file to the correct derivative path.
def write!
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be the best place for this switching logic; I suspect there's another class that's somewhat itching to emerge.

@jeremyf jeremyf force-pushed the derivative-locator branch 2 times, most recently from 0d323b1 to 97a1354 Compare March 17, 2023 16:35
@jeremyf jeremyf added the patch-ver for release notes label Mar 17, 2023
jeremyf added a commit that referenced this pull request Mar 17, 2023
jeremyf added a commit that referenced this pull request Mar 17, 2023
This is preliminary modeling for imports to handle associating existing
derivatives with a `Bulkrax::Entry`.

Hyrax has implicit derivative_types that are not declared, but in part
assumed as a general list of derivatives.  In this model, we want to
begin name those derivatives to address the following use cases:

```gherkin
Given a FileSet
When I provide a thumbnail derivative
Then I want to add that as the thumbnail for the FileSet

Given a FileSet
When I do not provide a thumbnail derivative
Then I want to generate a thumbnail
And add the generated as the thumbnail for the FileSet
```

Related to:

- #761
- #760
@jeremyf jeremyf marked this pull request as ready for review March 17, 2023 18:10
@jeremyf jeremyf added ignore-for-release ignore this for release notes and removed needs discussion patch-ver for release notes labels Mar 17, 2023
jeremyf added a commit that referenced this pull request Mar 17, 2023
This is preliminary modeling for imports to handle associating existing
derivatives with a `Bulkrax::Entry`.

Hyrax has implicit derivative_types that are not declared, but in part
assumed as a general list of derivatives.  In this model, we want to
begin name those derivatives to address the following use cases:

```gherkin
Given a FileSet
When I provide a thumbnail derivative
Then I want to add that as the thumbnail for the FileSet

Given a FileSet
When I do not provide a thumbnail derivative
Then I want to generate a thumbnail
And add the generated as the thumbnail for the FileSet
```

Related to:

- #761
- #760
- notch8/utk-hyku#371
@jeremyf jeremyf force-pushed the derivative-locator branch from 97a1354 to 803980f Compare March 20, 2023 17:46
jeremyf added a commit that referenced this pull request Mar 20, 2023
This is preliminary modeling for imports to handle associating existing
derivatives with a `Bulkrax::Entry`.

Hyrax has implicit derivative_types that are not declared, but in part
assumed as a general list of derivatives.  In this model, we want to
begin name those derivatives to address the following use cases:

```gherkin
Given a FileSet
When I provide a thumbnail derivative
Then I want to add that as the thumbnail for the FileSet

Given a FileSet
When I do not provide a thumbnail derivative
Then I want to generate a thumbnail
And add the generated as the thumbnail for the FileSet
```

Related to:

- #761
- #760
- notch8/utk-hyku#371
@jeremyf jeremyf force-pushed the derivative-locator branch from 803980f to 09190f2 Compare March 20, 2023 18:13
Prior to this commit, I was passing an array of strings back from the
locator.  With that pathway, I was ignoring how the existing
`Hyrax::FileSetDerivativesService` would likely be used as a fallback.

With this commit, I'm beginning to account for how that interaction
would be; in part by focusing on a known use case and building the
interactions accordingly.

This commit stencils in the expected behavior in what I think is
adequate to convey intention and design.

Related to:

- #760
- notch8/utk-hyku#343
@jeremyf jeremyf force-pushed the derivative-locator branch from 09190f2 to 42df4df Compare March 20, 2023 18:18
With this commit, I begin demonstrating how the original configuration
can be leveraged to do the derivative work.  This still assumes a
configuration for derivatives; something that I continue to punt on
until I have more concrete code that I can further build from.

Related to:

- #760
- notch8/utk-hyku#343
@jeremyf jeremyf changed the title Adding Samvera::Derivatives for interface discussion Adding Samvera::Derivatives interface and Hyrax shim Mar 20, 2023
@jeremyf jeremyf added patch-ver for release notes and removed ignore-for-release ignore this for release notes labels Mar 20, 2023
jeremyf added a commit that referenced this pull request Mar 20, 2023
With this commit, I begin to take the work already done in [this PR][1]
and start pseudo-coding my approach.

All of this continues to circle around the configuration for a
downstream implementation.  However, it's drawing closer.

[1]: #761
jeremyf added a commit that referenced this pull request Mar 20, 2023
With this commit, I begin to take the work already done in [this PR][1]
and start pseudo-coding my approach.

All of this continues to circle around the configuration for a
downstream implementation.  However, it's drawing closer.

[1]: #761
jeremyf added a commit that referenced this pull request Mar 20, 2023
With this commit, I begin to take the work already done in [this PR][1]
and start pseudo-coding my approach.

All of this continues to circle around the configuration for a
downstream implementation.  However, it's drawing closer.

[1]: #761
@jeremyf jeremyf force-pushed the derivative-locator branch from 60f5c32 to 194d307 Compare March 20, 2023 21:01
@jeremyf jeremyf force-pushed the derivative-locator branch from 194d307 to f9d143b Compare March 20, 2023 21:04
jeremyf added a commit that referenced this pull request Mar 22, 2023
With this commit, I begin to take the work already done in [this PR][1]
and start pseudo-coding my approach.

All of this continues to circle around the configuration for a
downstream implementation.  However, it's drawing closer.

[1]: #761
@orangewolf
Copy link
Member

orangewolf commented Mar 23, 2023

@jeremyf I do think having this work in Hyrax is the right thing. I get doing it in bulkrax temporarily, but I think it needs to be very temporary. The process should work if you are direct creating the items in the interface and it should hook in to the actor stack. Bulkrax flow should know nothing about it at all because it just passes the correctly formatted thing to the actor stack.

@orangewolf
Copy link
Member

I’m curious why this isn’t happening in iiif-print where we already massively modify the derivative process?

@jeremyf
Copy link
Contributor Author

jeremyf commented Mar 23, 2023

Moving this to IIIF Print

@jeremyf jeremyf closed this Mar 23, 2023
@jeremyf
Copy link
Contributor Author

jeremyf commented Mar 23, 2023

@orangewolf Bulkrax needs to know about it in that it needs a mechanism for capturing remote URLs; perhaps there's no necessary knowledge but those are not quite related to Bulkrax.

@jeremyf jeremyf deleted the derivative-locator branch March 23, 2023 18:03
jeremyf added a commit to notch8/iiif_print that referenced this pull request Mar 23, 2023
Prior to this commit, I was passing an array of strings back from the
locator.  With that pathway, I was ignoring how the existing
`Hyrax::FileSetDerivativesService` would likely be used as a fallback.

With this commit, I'm beginning to account for how that interaction
would be; in part by focusing on a known use case and building the
interactions accordingly.

This commit stencils in the expected behavior in what I think is
adequate to convey intention and design.

I also begin demonstrating how the original configuration can be
leveraged to do the derivative work.  This still assumes a configuration
for derivatives; something that I continue to punt on until I have more
concrete code that I can further build from.

Related to:

- samvera/bulkrax#760
- samvera/bulkrax#761
- notch8/utk-hyku#343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants