samvera / serverless-iiif

IIIF Image API 2.1 & 3.0 server in an AWS Serverless Application
https://samvera.github.io/serverless-iiif/
Apache License 2.0
69 stars 21 forks source link

Implement IIIFv3 and Lambda response streaming #114

Closed mbklein closed 1 year ago

mbklein commented 1 year ago
mbklein commented 1 year ago

A preview of the changes to the documentation is available at: http://site-previews.samvera.org/serverless-iiif/v5.0-streaming-iiif3/index.html

NextJS and S3 static site hosting don't work real well together, so you always have to enter through the index page. Deep linking and refreshing after navigation will break things. But the relevant sections for this PR can be found by navigating to:

orangewolf commented 1 year ago

I may have figured this out. My node_modules in src directory was stale.

I've got it deployed and I can see the layer is included BUT I can not process any images due to the following issue:

Screenshot 2023-08-18 at 17 14 42
2023-08-19T00:01:07.531Z    undefined   ERROR   Uncaught Exception  
{
    "errorType": "Error",
    "errorMessage": "\nSomething went wrong installing the \"sharp\" module\n\nCannot find module '../build/Release/sharp-linux-x64.node'\nRequire stack:\n- /var/task/node_modules/sharp/lib/sharp.js\n- /var/task/node_modules/sharp/lib/constructor.js\n- /var/task/node_modules/sharp/lib/index.js\n- /var/task/node_modules/iiif-processor/lib/transform.js\n- /var/task/node_modules/iiif-processor/index.js\n- /var/task/index.js\n- /var/runtime/index.mjs\n\nPossible solutions:\n- Install with verbose logging and look for errors: \"npm install --ignore-scripts=false --foreground-scripts --verbose sharp\"\n- Install for the current linux-x64 runtime: \"npm install --platform=linux --arch=x64 sharp\"\n- Consult the installation documentation: https://sharp.pixelplumbing.com/install",
    "stack": [
        "Error: ",
        "Something went wrong installing the \"sharp\" module",
        "",
        "Cannot find module '../build/Release/sharp-linux-x64.node'",
        "Require stack:",
        "- /var/task/node_modules/sharp/lib/sharp.js",
        "- /var/task/node_modules/sharp/lib/constructor.js",
        "- /var/task/node_modules/sharp/lib/index.js",
        "- /var/task/node_modules/iiif-processor/lib/transform.js",
        "- /var/task/node_modules/iiif-processor/index.js",
        "- /var/task/index.js",
        "- /var/runtime/index.mjs",
        "",
        "Possible solutions:",
        "- Install with verbose logging and look for errors: \"npm install --ignore-scripts=false --foreground-scripts --verbose sharp\"",
        "- Install for the current linux-x64 runtime: \"npm install --platform=linux --arch=x64 sharp\"",
        "- Consult the installation documentation: https://sharp.pixelplumbing.com/install",
        "    at Object.<anonymous> (/var/task/node_modules/sharp/lib/sharp.js:31:9)",
        "    at Module._compile (node:internal/modules/cjs/loader:1256:14)",
        "    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)",
        "    at Module.load (node:internal/modules/cjs/loader:1119:32)",
        "    at Module._load (node:internal/modules/cjs/loader:960:12)",
        "    at Module.require (node:internal/modules/cjs/loader:1143:19)",
        "    at require (node:internal/modules/cjs/helpers:110:18)",
        "    at Object.<anonymous> (/var/task/node_modules/sharp/lib/constructor.js:8:1)",
        "    at Module._compile (node:internal/modules/cjs/loader:1256:14)",
        "    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)"
    ]
}
t4k commented 1 year ago

I'd love to see this released this week if possible to spin up a new application and meet an end-of-the-month deadline! ❤️

github-actions[bot] commented 1 year ago

PR Preview Action v1.4.4 :---: Preview removed because the pull request was closed. 2023-08-31 22:19 UTC

t4k commented 1 year ago

I've been following along very closely trying to plan a deployment when this is ready. Thank you for the example template for CloudFormation. This may be the wrong place to ask since you are in the middle of working on it, but it does relate to the new docs. In reading through and trying to understand, I'm not sure how to distinguish the CacheDomainName and CacheHostName parameters. I see them joined together like !Sub "${CacheHostName}.${CacheDomainName}" in the Aliases property of the CloudFront Distribution. I think I would normally fill in on the UI with my full domain name like images.archives.example.org. In this template would I be splitting them up like images.archives for the host and example.org for the domain? Again, sorry if this is the wrong place to ask.

mbklein commented 1 year ago

Before I merge this, I have one more request. I decided, based on some developer conversations, that it would be best to keep the CloudFront wrapper in the next release, but with a deprecation notice and examples in the README (the one that displays in the Serverless App Repo, which will now be separate from the one in the project root). A second (or third, or fourth) set of eyes on these changes would be extremely helpful.

The files that need vetting are:

Anywhere any of those point to directories within https://github.com/samvera/serverless-iiif/tree/main, the PR content is at https://github.com/samvera/serverless-iiif/tree/v5.0-streaming-iiif3. (I couldn’t make them relative links because they’re going to be published to the AWS App Repository.

We also have a great new doc site created by @adamjarling. It’s already deployed off main, but there are significant changes to it in this PR. So anything in the above READMEs pointing to https://samvera.github.io/serverless-iiif/ is actually rooted at https://samvera.github.io/serverless-iiif/pr-preview/pr-114/.

@orangewolf @t4k @tpendragon @edsilv @codeclout @fitnycdigitalinitiatives @theiawolfe

mbklein commented 1 year ago

I'm not sure how to distinguish the CacheDomainName and CacheHostName parameters.

If you take a look at these two lines in the CloudFormation template, you'll see the issue. Route53 wants both the FQDN of the record you're creating, as well as the name of the zone it's hosted in.

For images.archives.example.org, there's no way to tell automatically whether the hosted zone is archives.example.org or example.org. (It's safe to assume it's not .org 😄) So by asking for the host and domain parts separately, the template is able to construct both parameters that it needs.

Obviously this is just an example template. You're free to ask for and use those pieces however you need you. You could ask for the full name (images.archives.example.org) in one parameter, and the hosted zone name or even the hosted zone ID in another, and construct the resource accordingly. The great thing about swapping out the bespoke CloudFront variant in favor of encouraging people to use existing tools is that, in addition to eventually reducing the maintenance burden of this project, it creates a ton of flexibility on how the IIIF “superstructure” is deployed.

t4k commented 1 year ago

A second (or third, or fourth) set of eyes on these changes would be extremely helpful.

I've read through the new documents and deprecation warnings and they make sense. My only concern as an end user would be what would happen to a CloudFront-enabled 5.0 version when the deprecated wrapper code is removed in 5.1 or whenever it is. That could certainly be documented more clearly in the release notes for that future version, but the upgrade path or its pitfalls are not obvious to me.

(Also, thank you for the response above, @mbklein. I see what is happening now. I glossed over the Route53 part because I don't use that service and instead talk to campus IT about all my DNS needs.)

orangewolf commented 1 year ago

I went over the latest readme changes and the combination of edits around build/deployment. I did my best to run an ignorant install and that worked well. Like @t4k I don't always have DNS control and both of our production sites would require met to talk to clients who then talk to campus IT, so I only testing DNS stuff against a sample domain.