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

Add client_id, s3, scope to bulk availability API #7522

Merged
merged 4 commits into from
Jan 19, 2024

Conversation

mekarpeles
Copy link
Member

In response to (internal) Bulk Availability API PR https://git.archive.org/ia/petabox/-/merge_requests/3516/diffs

This PR decorates our calls to the Bulk Availability API (petabox) to include headers with s3 authorization, scope, application, and client_ip to help enforce security/permissions and rate limiting.

Technical

The scope field may need to be changed? I'm not sure what values are acceptable.

Testing

This PR can be deployed to testing when there's a petabox PR to test against

Stakeholders

@ximm @cdrini

@mekarpeles mekarpeles requested a review from ximm February 10, 2023 21:23
mekarpeles and others added 2 commits July 20, 2023 07:09
Co-authored-by: Christian Clauss <cclauss@me.com>
@mekarpeles mekarpeles marked this pull request as ready for review January 19, 2024 19:19
@mekarpeles mekarpeles added the Patch Deployed This PR has been deployed to production independently, outside of the regular deploy cycle. label Jan 19, 2024
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Code lgtm; not testing. @mekarpeles and @ximm have tested. Expediting to unblock testing #8732

@cdrini cdrini merged commit 49ee282 into master Jan 19, 2024
@cdrini cdrini deleted the s3-keys-scope-bulk-availability branch January 19, 2024 19:25
Achorn pushed a commit to Achorn/openlibrary that referenced this pull request Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Patch Deployed This PR has been deployed to production independently, outside of the regular deploy cycle.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants