Skip to content

feat: Include starting offset for GHAC upload Content-Range#3163

Merged
Xuanwo merged 1 commit intoapache:mainfrom
huonw:huonw/3162-ghac-partial
Sep 23, 2023
Merged

feat: Include starting offset for GHAC upload Content-Range#3163
Xuanwo merged 1 commit intoapache:mainfrom
huonw:huonw/3162-ghac-partial

Conversation

@huonw
Copy link
Copy Markdown
Contributor

@huonw huonw commented Sep 23, 2023

This adjusts the GitHub Action Cache service to handle multi-part uploads correctly: each upload part needs to specify exactly the range it is for, e.g. to upload an 11 byte file with parts <= 8 bytes, the two requests should set:

  1. Content-Range: bytes 0-8/* (first 8 bytes)
  2. Content-Range: bytes 9-10/* (final 3 bytes)

Previously, the second request would be Content-Range: 0-2/*. This would leave the file not-fully written, and GitHub would reject it with an error when committing (close()-ing the writer).

Fixes #3162

This adjusts the GitHub Action Cache service to handle multi-part
uploads correctly: each upload part needs to specify exactly the range
it is for, e.g. to upload an 11 byte file with parts <= 8 bytes, the
two requests should set:

1. `Content-Range: bytes 0-8/*` (first 8 bytes)
2. `Content-Range: bytes 9-10/*` (final 3 bytes)

Previously, the second request would be `Content-Range: 0-2/*`. This
would leave the file not-fully written, and GitHub would reject it
with an error.

Fixes apache#3162
@huonw huonw requested a review from Xuanwo as a code owner September 23, 2023 03:52
@huonw
Copy link
Copy Markdown
Contributor Author

huonw commented Sep 23, 2023

There doesn't appear to be automatic tests for GHAC? Let me know if I've missed them.

I've tested this in GHA:

Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Great, thanks a lot!

@Xuanwo Xuanwo changed the title Include starting offset for GHAC upload Content-Range feat: Include starting offset for GHAC upload Content-Range Sep 23, 2023
@github-actions github-actions Bot added the releases-note/feat The PR implements a new feature or has a title that begins with "feat" label Sep 23, 2023
@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented Sep 23, 2023

There doesn't appear to be automatic tests for GHAC? Let me know if I've missed them.

GHAC requires github secrets which can't be fetched from a forked repository. This is a github issue. You have already done the great work!

@Xuanwo Xuanwo merged commit a2fab34 into apache:main Sep 23, 2023
@huonw huonw deleted the huonw/3162-ghac-partial branch September 23, 2023 10:14
huonw pushed a commit to pantsbuild/pants that referenced this pull request Oct 22, 2023
The pull requests apache/opendal#3163
and apache/opendal#3177 have been
merged and included in OpenDAL version 0.41. This PR will update OpenDAL
to version 0.41, allowing us to rely on tags rather than a specific
GitHub commit.

Signed-off-by: GitHub <noreply@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/feat The PR implements a new feature or has a title that begins with "feat"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GHAC multi-part writes only take the last part, causing commit errors

2 participants