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

[WJ-1196] Add support for S3 presign URLs to upload blobs #1918

Draft
wants to merge 73 commits into
base: develop
Choose a base branch
from

Conversation

emmiegit
Copy link
Member

@emmiegit emmiegit commented May 12, 2024

This PR changes the upload process to use S3 presign URLs. See the Jira issue to understand why we are changing the upload system.

This PR has a number of corresponding changes:

  • Creates a blob_pending table to track in-progress uploads. This table will use cuids to generate external IDs, like other non-public data scopes such as messages.
  • Adds BlobService methods for creating, cancelling, and finishing pending blob uploads. The last is not in the JSONRPC API but is instead called by the relevant services instead.
  • Methods which previously wanted file data to upload now want a pending blob upload ID. This should correspond to an uploaded blob, or it will yield an error to the user. Any methods which attempt to make a change but fail require a new pending blob and upload to be performed.
  • Adds configuration fields for upload-related settings.
  • Adds new errors for blob-related issues.
  • Adds instructions to the DEEPWELL README explaining how to locally call requests and how to upload files to the generated presign URLs.
  • Adds an echo API request, for testing purposes.
  • Fixes a bug in ProvidedValue that prevents it from being used to serialize data.
  • DEEPWELL version is bumped.

Copy link

codecov bot commented Sep 30, 2024

Codecov Report

Attention: Patch coverage is 3.38983% with 171 lines in your changes missing coverage. Please review.

Project coverage is 1.98%. Comparing base (82201c7) to head (24b4218).
Report is 46 commits behind head on develop.

Files with missing lines Patch % Lines
deepwell/src/services/blob/service.rs 0.00% 85 Missing ⚠️
deepwell/src/services/file/service.rs 0.00% 25 Missing ⚠️
deepwell/src/endpoints/blob.rs 0.00% 20 Missing ⚠️
deepwell/src/services/file_revision/service.rs 0.00% 12 Missing ⚠️
deepwell/src/services/user/service.rs 0.00% 9 Missing ⚠️
deepwell/src/config/file.rs 0.00% 5 Missing ⚠️
deepwell/src/endpoints/misc.rs 0.00% 4 Missing ⚠️
deepwell/src/endpoints/file.rs 0.00% 3 Missing ⚠️
deepwell/src/services/error.rs 0.00% 3 Missing ⚠️
deepwell/src/models/blob_pending.rs 0.00% 2 Missing ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #1918      +/-   ##
==========================================
+ Coverage     1.94%   1.98%   +0.03%     
==========================================
  Files          137     139       +2     
  Lines         5447    5646     +199     
==========================================
+ Hits           106     112       +6     
- Misses        5341    5534     +193     
Flag Coverage Δ
deepwell 1.98% <3.38%> (+0.03%) ⬆️
Files with missing lines Coverage Δ
deepwell/src/api.rs 0.00% <ø> (ø)
deepwell/src/config/object.rs 0.00% <ø> (ø)
deepwell/src/services/email/structs.rs 0.00% <ø> (ø)
deepwell/src/services/import/service.rs 0.00% <ø> (ø)
deepwell/src/web/provided_value.rs 33.33% <100.00%> (+33.33%) ⬆️
deepwell/src/services/page_revision/service.rs 0.00% <0.00%> (ø)
deepwell/src/models/blob_pending.rs 0.00% <0.00%> (ø)
deepwell/src/models/user.rs 0.00% <0.00%> (ø)
deepwell/src/endpoints/file.rs 0.00% <0.00%> (ø)
deepwell/src/services/error.rs 0.00% <0.00%> (ø)
... and 7 more

... and 2 files with indirect coverage changes

Not sure how this was missed, I thought it was already here.
How did I not catch this before? lol
Apparently I missed this. Good thing to test for, given that
we can return it now for blob_created.
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.

1 participant