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

Revert "chore: Stream S3 files directly to zip" #2469

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Nov 21, 2023

Reverts #2380 - building this pizza again to see if it introduced issue described here (update: it did): https://opendigitalplanning.slack.com/archives/C0241GWFG4B/p1700561792278499

Fixes issue where user-uploaded files appear not to be included in zip reported in #help-issues. On production, I can still see a /tmp folder → {sessionId}_{s3FolderId} folder → user uploaded files, but it sounds like for MS users the /tmp folder is being hidden altogether !

This one was a small tidy up without major performance impacts, so just going to revert & deploy fix for now & will add a Trello ticket to pick back up and solve properly 👌

Copy link

github-actions bot commented Nov 21, 2023

Removed vultr server and associated DNS entries

@jessicamcinchak jessicamcinchak marked this pull request as ready for review November 21, 2023 11:36
@jessicamcinchak jessicamcinchak requested a review from a team November 21, 2023 11:36
Copy link
Contributor

@Mike-Heneghan Mike-Heneghan left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Some things I was surprised about but I don't think are anything to do with this piece of work.

The Overview.htm and RedactedOverview.htm are name with .htm rather than .html

I couldn't see how the labels I added to the files could be associated with the uploaded files from what I could see in the zip.

@jessicamcinchak
Copy link
Member Author

Thanks @Mike-Heneghan !

  • htm versus html is a very strange Uniform DMS requirement 🙈 (but either should behave same when opened in your browser)
  • Labels are only currently shown/associated to files in the BOPS payload (and forthcoming ODP Schema), this is another known limitation of OneApp XM

@jessicamcinchak jessicamcinchak merged commit 047c8b7 into main Nov 21, 2023
12 checks passed
@jessicamcinchak jessicamcinchak deleted the revert-2380-dp/stream-directly-from-s3-to-zip branch November 21, 2023 12:00
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