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 StageBlockWithURL isn't implemented #2440

Open
radekg opened this issue Jul 30, 2024 · 6 comments
Open

Block blob StageBlockWithURL isn't implemented #2440

radekg opened this issue Jul 30, 2024 · 6 comments
Assignees
Labels
blob-storage featureparity Tracking issues for catching up feature parity

Comments

@radekg
Copy link

radekg commented Jul 30, 2024

Which service(blob, file, queue, table) does this issue concern?

This ticket is for the blob service.

Which version of the Azurite was used?

Version: 2024.04 Version 3.30.0.

Where do you get Azurite? (npm, DockerHub, NuGet, Visual Studio Code Extension)

DockerHub

What's the Node.js version?

Running in Docker, irrelevant.

What problem was encountered?

RESPONSE 500: 500 Current API is not implemented yet. Please vote your wanted features to https://github.com/azure/azurite/issues
ERROR CODE: APINotImplemented
--------------------------------------------------------------------------------
<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>
<Error>
  <Code>APINotImplemented</Code>
  <Message>Current API is not implemented yet. Please vote your wanted features to https://github.com/azure/azurite/issues\nRequestId:8acf1d5b-511f-4c13-8044-af31e423b2ae\nTime:2024-07-30T08:03:49.048Z</Message>
</Error>
--------------------------------------------------------------------------------

Steps to reproduce the issue?

Issue a request to blockblob client StageBlockWithURL(...).

public async stageBlockFromURL(
blockId: string,
contentLength: number,
sourceUrl: string,
options: Models.BlockBlobStageBlockFromURLOptionalParams,
context: Context
): Promise<Models.BlockBlobStageBlockFromURLResponse> {
throw new NotImplementedError(context.contextId);
}

Have you found a mitigation/solution?

There's no solution.

@radekg radekg changed the title Block bloc client StageBlockWithURL isn't implemented Block blob StageBlockWithURL isn't implemented Jul 30, 2024
@radekg
Copy link
Author

radekg commented Jul 30, 2024

This is how I'd Implement it: main...radekg:Azurite:blockBlobClient-stageBlockFromURL.

Some work would have to be done to extract those functions copied straight from BlobHandler.ts. But the supplied test passes. Would my branch be a good starting point, or am way off regarding how it should be implemented?

@blueww blueww self-assigned this Jul 31, 2024
@blueww blueww added blob-storage featureparity Tracking issues for catching up feature parity labels Jul 31, 2024
@radekg
Copy link
Author

radekg commented Jul 31, 2024

The Docker image built from my branch does what it supposed to do. I can put more work in this to have it merged but I'd need guidance. Thank you.

@blueww
Copy link
Member

blueww commented Jul 31, 2024

@radekg

Thanks for the contribution!
Would you please indicate the detail support you need from us to prepare this PR to implement stageblockfromURL?

BTW, Azurite blob service support 2 kinds of data location (loki and SQL), not sure if you would like also support SQL?
If so, have you tested both work?
If not, Readme.md should be updated for the support limitation. And test case should not cover sql.

Besides that, you might can follow up rest API doc , and make sure the rest API doc mentioned error / details behavior is aligned with Azurite implementation, and add enough test cases if necessary.
For some details scenarios, sometime we need to test on public Azure server to get the real server behavior and make Azurite aligned.

@radekg
Copy link
Author

radekg commented Jul 31, 2024

@blueww I have opened the PR. Regarding the guidance I need:

Shall we continue the discussion on the PR?

@blueww
Copy link
Member

blueww commented Aug 2, 2024

@radekg

Thanks for the contribution!

It's not good to make have duplicated copy of almost same code. It will make the maintenance of the code be difficult.
For the common Util function used in several place in blob handler, we put it into this file: https://github.com/Azure/Azurite/blob/main/src/blob/utils/utils.ts
Not sure if you have move the function to this file and make both handler use the function in this file?

It's OK if SQL will not be supported at the first version of the API implementation. You just need update Readme.md to add the limitation like : https://github.com/Azure/Azurite#:~:text=Copy%20Blob%20From%20URL%20(Only%20supports%20copy%20within%20same%20Azurite%20instance%2C%20only%20on%20Loki)

@radekg
Copy link
Author

radekg commented Aug 4, 2024

Hey @blueww, I have updated the PR. I moved those shared functions to blob/utils/utils.go. Not sure how you'd go about the logger and loggerPrefix arguments, there's never a good way to generalize such code with caller-dependent log messages. I have also updated the readme and tagged the test for @loki only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blob-storage featureparity Tracking issues for catching up feature parity
Projects
None yet
Development

No branches or pull requests

2 participants