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

PDF file sets should not have a pyramid location #4391

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

kdid
Copy link
Contributor

@kdid kdid commented Feb 5, 2025

Summary

On the preservation report, Auxiliary files of type PDF were erroring because the pyramid was not found.

Specific Changes in this PR

  • Update Meadow.Data.FileSets.pyramid_uri_for() so that it does not return a pyramid location for a file set with mime-type of application/pdf

Version bump required by the PR

See Semantic Versioning 2.0.0 for help discerning which is required.

  • Patch
  • Minor
  • Major

Steps to Test

  • Check preservation report for a pdf Auxiliary file.
  • The pyramid_location column should be empty and the pyramid_exists column value should be N/A

🚀 Deployment Notes

Note - if you check any of these boxes go to the (always open) main <- staging PR and add detailed notes and instructions to help out others who may end up deploying your changes to production

  • Backward compatible API changes
    • Database Schema changes
    • GraphQL API
    • Elasticsearch API
    • Ingest Sheet
    • CSV metadata export/update API
    • Shared Links export API
  • Backwards-incompatible API changes
    • Database Schema changes
    • GraphQL API
    • Elasticsearch API
    • Ingest Sheet
    • CSV metadata export/update API
    • Shared Links export API
  • Requires data migration
  • Requires database triggers disabled during deployment/migration
  • Requires reindex
  • Terraform changes
    • Adds/requires new or changed Terraform variables
  • Pipeline configuration changes (requires mix meadow.pipeline.setup run)
  • Requires new variable added to miscellany
  • Specific deployment synchronization instructions with other apps/API's
  • Other specific instructions/tasks

Tested/Verified

  • End users/stakeholders

@kdid kdid self-assigned this Feb 5, 2025
@kdid kdid requested a review from mbklein February 5, 2025 18:50
Copy link
Contributor

@mbklein mbklein left a comment

Choose a reason for hiding this comment

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

Approving this as the least invasive way to make this work, but I wonder if instead of excluding specific types from having pyramids, we should only include it for the specific types that need them? (e.g., image/* and anything with a zoomable thumbnail?)

@kdid kdid force-pushed the 5379-pres-report branch from 5aa6c3b to a226e85 Compare February 6, 2025 14:17
@kdid
Copy link
Contributor Author

kdid commented Feb 6, 2025

@mbklein - good catch. I made an adjustment. Thanks.

@kdid kdid merged commit be8c46b into deploy/staging Feb 6, 2025
3 checks passed
@kdid kdid deleted the 5379-pres-report branch February 6, 2025 14:28
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.

2 participants