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

[i335] - port over serverless code from nnp #198

Merged
merged 6 commits into from
Mar 24, 2023

Conversation

ShanaLMoore
Copy link
Contributor

@ShanaLMoore ShanaLMoore commented Mar 22, 2023

Summary

ref: notch8/britishlibrary#335

This PR allows iiif_print to support iiif external image servers

Screenshots / Video

Screenshot 2023-03-23 at 09 49 57

Expected Behavior

  • All files should get served from cloudfront instead of the rails app.

TODO:

There's a loading issue in dev; when we change code the app no longer hits our iiif_manifest_presenter_behavior.rb it reverts back to using the app instead of cloudfront. This seems to only impact dev but it's really annoying.

We also added the DisplayImagePresenterBehavior prepend in the after_intitialize because the app couldn't find the digest method otherwise.

We will also need to figure out how to make serverless iiif work for audio/video files.

Shana Moore and others added 4 commits March 21, 2023 16:47
…terBehavior

TODO: Loading issue in dev; when we change code the app no longer hits our iiif_manifest_presenter_behavior.rb and reverts back to using the app instead of cloudfront. We also added the prepend in the after_intitialize because the app couldn't find the digest method otherwise.
This commit will add a test for #display_image.  Also, a refactor was
introduced to account for both Hyrax 2 and 3.  The method signature
of the `Hyrax.config.iiif_image_url_builder` lambda changed to add an
additional argument in Hyrax 3.

ref: https://github.com/samvera/hyrax/blob/2.x-stable/lib/hyrax/configuration.rb#L397-L399
ref: https://github.com/samvera/hyrax/blob/3.x-stable/lib/hyrax/configuration.rb#L250-L252
@ShanaLMoore ShanaLMoore marked this pull request as ready for review March 23, 2023 19:09
@ShanaLMoore ShanaLMoore requested review from jeremyf, laritakr and orangewolf and removed request for jeremyf March 23, 2023 19:10
Copy link
Contributor

@orangewolf orangewolf left a comment

Choose a reason for hiding this comment

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

This looks great. 2 small calls for documentation and 1 change of heart on a name. The name thing didn't even occur to me until today. Sorry about that.

@ShanaLMoore ShanaLMoore marked this pull request as draft March 23, 2023 20:40
@kirkkwang kirkkwang force-pushed the i335-import-nnp-serverless-code-into-iiif-print branch 2 times, most recently from 3d31203 to ac3edcf Compare March 23, 2023 23:21
@ShanaLMoore ShanaLMoore marked this pull request as ready for review March 23, 2023 23:38
This commit will add OVERRIDE comments where useful and add a blurb in
the README for this new feature.  The #display_content method was added
to support presentation 3 manifests.  `SERVERLESS` has been renamed to
`EXTERNAL`.  `DisplayImagePresenterBehavior` has beeen refactored to
incorporate existing code.  Specs have also been refactored.
@kirkkwang kirkkwang force-pushed the i335-import-nnp-serverless-code-into-iiif-print branch from ac3edcf to 4a08e55 Compare March 24, 2023 13:53
@ShanaLMoore ShanaLMoore merged commit cd0d582 into main Mar 24, 2023
@orangewolf orangewolf deleted the i335-import-nnp-serverless-code-into-iiif-print branch April 23, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants