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

Block blob client stage block from url #2442

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

radekg
Copy link

@radekg radekg commented Jul 31, 2024

This PR implements blockBlob stageBlockFromURL and relates to #2440.

Add Change Log

  • Implemented blockBlob stageBlockFromURL.

@radekg
Copy link
Author

radekg commented Jul 31, 2024

@microsoft-github-policy-service agree company="Klarrio GmbH"

@blueww blueww self-requested a review August 2, 2024 06:38
@blueww
Copy link
Member

blueww commented Aug 2, 2024

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

README.md Outdated Show resolved Hide resolved
@blueww
Copy link
Member

blueww commented Aug 6, 2024

@radekg

Thanks for the quick fix!

I might still need more time to test and review this PR.
Will update you later.

@@ -270,7 +270,78 @@ export default class BlockBlobHandler
options: Models.BlockBlobStageBlockFromURLOptionalParams,
context: Context
): Promise<Models.BlockBlobStageBlockFromURLResponse> {
throw new NotImplementedError(context.contextId);

Copy link
Member

@blueww blueww Aug 23, 2024

Choose a reason for hiding this comment

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

The implementation doesn't respect header "x-ms-source-range". (rest API doc)
The header is important, since normally this API will be used to copy blob block blob (For small blob less then 256MB, can use single CopyFromUrl() API.) And for big blob, normally the copy should happen chunk by chunk, so customer can use "x-ms-source-range" to handle which chunk of source blob will be copy.

throw StorageErrorFactory.getConditionNotMet(context.contextId!);
}

const stageBlockRes = await this.stageBlock(blockId,
Copy link
Member

@blueww blueww Aug 23, 2024

Choose a reason for hiding this comment

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

When copy a 300000KB block blob, will get error here:

2024-08-23T09:40:50.503Z 8f01df46-0ea6-4ade-9594-7a46dec001a7 error: ErrorMiddleware: ErrorName=StorageError ErrorMessage=The size of the request body 307167232 mismatches the content-length 307200000. ErrorHTTPStatusCode=400 ErrorHTTPStatusMessage=The size of the request body 307167232 mismatches the content-length 307200000. ErrorHTTPHeaders={"x-ms-error-code":"InvalidOperation","x-ms-request-id":"8f01df46-0ea6-4ade-9594-7a46dec001a7"} ErrorHTTPBody="\n\n InvalidOperation\n The size of the request body 307167232 mismatches the content-length 307200000.\nRequestId:8f01df46-0ea6-4ade-9594-7a46dec001a7\nTime:2024-08-23T09:40:17.319Z\n" ErrorStack="StorageError: The size of the request body 307167232 mismatches the content-length 307200000.\n at Function.getInvalidOperation (D:\code\azurite\src\blob\errors\StorageErrorFactory.ts:151:12)\n at BlockBlobHandler.stageBlock (D:\code\azurite\src\blob\handlers\BlockBlobHandler.ts:203:33)\n at processTicksAndRejections (node:internal/process/task_queues:95:5)\n at async BlockBlobHandler.stageBlockFromURL (D:\code\azurite\src\blob\handlers\BlockBlobHandler.ts:324:27)"

Copy link
Member

@blueww blueww left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

I tried to test the change by Powershell cmdlet Copy-AzStorageBlob with a 300000KB block blob, it failed.
See details in my comments.

Besides fix the issue, please also rebase the code on latest main branch to get latest test fix, so the CI can pass.

@@ -70,6 +63,83 @@ describe("BlockBlobAPIs", () => {
await containerClient.delete();
});

it("Blob should be staged from URL and committed @loki", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Please also cover :

  1. "x-ms-source-range" when only copy part of source blob as block
  2. Big source blob like > 300MB
  3. Cross account copy (both account in same Azurite instance)

@@ -1010,6 +1010,7 @@ Detailed support matrix:
- Abort Copy Blob (Only supports copy within same Azurite instance)
- Copy Blob From URL (Only supports copy within same Azurite instance, only on Loki)
- Access control based on conditional headers
- Stage Block From URL (Only supports copy within same Azurite instance, only on Loki)
Copy link
Member

Choose a reason for hiding this comment

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

Please also update changelog

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