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

Implement IIIFv3 and Lambda response streaming #114

Merged
merged 6 commits into from
Aug 31, 2023
Merged

Conversation

mbklein
Copy link
Member

@mbklein mbklein commented Jul 14, 2023

  • Convert Lambda to streaming function url
  • Use IIIF v3 branch of node-iiif
  • Add IIIF v3 tests and other supports
  • Add service discovery endpoint at /
  • Create Terraform module
  • Create CloudFormation and Terraform examples
  • Enhance docs

@mbklein mbklein requested a review from orangewolf July 14, 2023 16:57
@mbklein mbklein force-pushed the v5.0-streaming-iiif3 branch 5 times, most recently from 1a9097d to 0ecd498 Compare July 19, 2023 15:54
@mbklein mbklein force-pushed the main branch 3 times, most recently from a0db911 to bfc172f Compare August 1, 2023 21:12
@mbklein mbklein force-pushed the v5.0-streaming-iiif3 branch 3 times, most recently from eea2fd0 to 76e12cd Compare August 9, 2023 15:19
@mbklein mbklein force-pushed the v5.0-streaming-iiif3 branch 2 times, most recently from 98303bb to 946d7e8 Compare August 18, 2023 15:12
@mbklein mbklein marked this pull request as ready for review August 18, 2023 15:12
@mbklein
Copy link
Member Author

mbklein commented Aug 18, 2023

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:

  • Read the Docs -> Quick Start -> Deploying via the Command Line
  • Read the Docs -> Quick Start -> Deploying via Infrastructure Tools

@orangewolf
Copy link
Member

orangewolf commented Aug 19, 2023

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)"
    ]
}

Formatting updates to markdown content

More style updates to homepage

Add new diagram image and update SEO

Add Solutions and Under the Hood sections to About page for ideas

Clean up styling on About page.  Add Step components to installation instructions

Add documentation deployment workflow
@mbklein mbklein changed the title Implment IIIFv3 and Lambda response streaming Implement IIIFv3 and Lambda response streaming Aug 22, 2023
- Convert Lambda to streaming function url
- Use IIIF v3 branch of node-iiif
- Add IIIF v3 tests and other supports
- Add service discovery endpoint at `/`
- Create Terraform module
- Create CloudFormation and Terraform examples
- Enhance docs
- Build CloudFormation & Terraform documentation dynamically
- Refactor build/deploy/delete commands to use new YAML config
- Update to NodeJS v18.x
- Update to aws-sdk v3

Co-Authored-By: Rob Kaufman <[email protected]>
Replace default "" with JP2
Expose PyramidLimit parameter for efficient ptiff handling
Copy link
Member

@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.

We completed compatibility testing and I think this is good to ship! Great work on this.

@t4k
Copy link

t4k commented Aug 27, 2023

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! ❤️

@mbklein mbklein force-pushed the v5.0-streaming-iiif3 branch 5 times, most recently from f550913 to e01f7f6 Compare August 28, 2023 17:19
@samvera samvera deleted a comment from github-actions bot Aug 28, 2023
mbklein added a commit that referenced this pull request Aug 28, 2023
@github-actions
Copy link

github-actions bot commented Aug 28, 2023

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

@t4k
Copy link

t4k commented Aug 28, 2023

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
Copy link
Member Author

mbklein commented Aug 28, 2023

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
Copy link
Member Author

mbklein commented Aug 28, 2023

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
Copy link

t4k commented Aug 28, 2023

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
Copy link
Member

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.

@mbklein mbklein merged commit 59db4e1 into main Aug 31, 2023
1 check passed
@mbklein mbklein deleted the v5.0-streaming-iiif3 branch August 31, 2023 22:17
mbklein added a commit that referenced this pull request Aug 31, 2023
Implement IIIFv3 and Lambda response streaming
mbklein added a commit that referenced this pull request Sep 1, 2023
Implement IIIFv3 and Lambda response streaming
mbklein added a commit that referenced this pull request Oct 23, 2023
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

Successfully merging this pull request may close these issues.

4 participants