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

Creates Blob Retrieval Endpoint #96

Merged
merged 6 commits into from
Jan 2, 2024
Merged

Creates Blob Retrieval Endpoint #96

merged 6 commits into from
Jan 2, 2024

Conversation

gbdubs
Copy link
Contributor

@gbdubs gbdubs commented Dec 28, 2023

The basic idea here is pretty simple: RMI has indicated to us that metadata is fairly nonsensitive, while the contents of the data (portfolios and reports) is hugely sensitive. My approach to this is to centralize the creation of download URLs in a single place (kind of the opposite of what we did for ADSCI's blob population logic). Additionally, we don't want to populate these proactively because RMI wants audit logs when people actually access the objects. Thus, here's my approach (which this PR starts to implement):

  • Have a common download button vue Component, which takes in one or more BlobIDs as the inputs.
  • When the download button is clicked, the component starts spinning (loading) it makes a request to our server for a download URL. The button is disabled, spinning during this request. It doesn't block page navigation.
  • On the BE, we record (transactionally) audit logs, then generate the download URLs. The ordering ensures our invariant of only granting access when audited. This is the endpoint that this PR implements.
  • Then, the download button will transition from spinning into a dial, using fetch and ReadableStream to increase the percentage done as the download (or downloads) complete.
  • Finally the button will turn into a check mark and remain disabled to prevent accidental duplicate downloads.

Thoughts?

@gbdubs gbdubs requested a review from bcspragu December 28, 2023 14:32
Copy link
Collaborator

@bcspragu bcspragu left a comment

Choose a reason for hiding this comment

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

LGTM!

cmd/server/pactasrv/blobs.go Outdated Show resolved Hide resolved
cmd/server/pactasrv/blobs.go Outdated Show resolved Hide resolved
cmd/server/pactasrv/blobs.go Outdated Show resolved Hide resolved
cmd/server/pactasrv/blobs.go Show resolved Hide resolved
cmd/server/pactasrv/blobs.go Outdated Show resolved Hide resolved
pacta/pacta.go Outdated Show resolved Hide resolved
@gbdubs gbdubs enabled auto-merge (squash) January 2, 2024 15:32
@gbdubs gbdubs merged commit b530d5d into main Jan 2, 2024
2 checks passed
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