Open mihaitodor opened 3 years ago
Small update: I tried running autorest --typescript --output-folder=./../src/blob/generated --clear-output-folder --input-file=blob-storage-2019-02-02.json
from the swagger
folder and got the following error:
AutoRest code generation utility [cli version: 3.0.6338; node: v10.23.3, max-memory: 1280 MB]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
NOTE: AutoRest core version selected from configuration: ~3.0.6298.
Loading AutoRest core '/root/.autorest/@autorest_core@3.0.6372/node_modules/@autorest/core/dist' (3.0.6372)
Loading AutoRest extension '@autorest/typescript' (6.0.0-dev.20201105.2->6.0.0-dev.20201105.2)
Loading AutoRest extension '@autorest/modelerfour' (4.15.421->4.15.421)
WARNING (PreCheck/SchemaMissingType): The schema 'DataLakeStorageError-error' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'
WARNING (PreCheck/SchemaMissingType): The schema 'Metrics' with an undefined type and decalared properties is a bit ambigious. This has been auto-corrected to 'type:object'
WARNING (PreCheck/CheckDuplicateSchemas): Checking for duplicate schemas, this could take a (long) while. Run with --verbose for more detail.
ERROR (): Duplicate object schemas with 'DataLakeStorageError' name detected.
FATAL: Error: 1 errors occured -- cannot continue.
I don't see any duplication of the DataLakeStorageError
schema in blob-storage-2019-02-02.json
, but maybe I'm doing something wrong.
@mihaitodor
Blob.AppendBlock gets resolved to AppendBlobHandler.create() instead of AppendBlobHandler.appendBlock().
[Wei] I am a little confused on this. Do you mean you would like to append block, but azurite actually created append blob? Per the rest doc of create blob and append block, you need add "?comp=appendblock" in the Uri to indicate you need append block. Have you add it when try to append block? Would you please share the Azurite debug log of the request with issue, and we will be more clear on this.
Would it be possible to make the x-ms-blob-type header mandatory for the AppendBlob_AppendBlock operation?
[Wei] per the rest doc of append block, it does not support x-ms-blob-type header. Why you want to add it, or even make it mandatory? This might will break other user of Azurite. Besides that, x-ms-blob-type is mandatory for create blob rest api.
Hey @blueww, thank you for looking into this issue and sharing links to the relevant documentation!
Here is the Azurite debug log (for v3.11.0)
In there, you can observe that DispatchMiddleware: Operation=AppendBlob_Create
is selected twice, although the second request URL ends with ?comp=appendblock
, so it never actually writes the content when blobRef.AppendBlock
is called in the code.
per the rest doc of append block, it does not support x-ms-blob-type header. Why you want to add it, or even make it mandatory? This might will break other user of Azurite. Besides that, x-ms-blob-type is mandatory for create blob rest api.
That's understandable, but I feel this REST API has been formalised after azure-sdk-for-go
was developed and they're not fully compatible. However, anyone who would like to test their existing code using Azurite won't be able to call AppendBlock
, as my code sample demonstrates.
My proposal to mark the x-ms-blob-type
header as mandatory for AppendBlock
(and AppendBlockFromUrl
) in Azurite is my naive attempt to getting Azurite to support this functionality when invoked from azure-sdk-for-go
, since I understand that library is unlikely to be changed given its legacy status. Unfortunately, I can't switch to the new storage SDK just yet as mentioned before.
I assumed that the lack of the x-ms-blob-type
header from the AppendBlock
and AppendBlockFromUrl
API calls is an omission, but maybe I'm not seeing some pattern. However, given that it's documented this way, I do agree that mandating it might break other client libraries, which is undesirable.
As an alternative, I put together a hack in #710, which is only active when the --loose
flag is set. If you think this is acceptable, it seems to do the job and shouldn't impact any other client libraries. Please let me know if you'd prefer to follow a different approach.
@mihaitodor As this is Go legacy SDK issue, it might not be so good to change Azurite for it. Would you like try to raise a PR for Go SDK repo to fix the issue? This might be a better way to fix the issue.
@blueww Unfortunately, that code is quite tangled and brittle. I fear changing this behaviour in it might cause unintended side-effects.
@mihaitodor
Would you please file an issue on Go SDK repo https://github.com/Azure/azure-sdk-for-go, and see if Go SDK owner can help to fix it.
Besides that, I will also try to ping the SDK owner to see if they can help to fix it.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Which service(blob, file, queue, table) does this issue concern?
blob
Which version of the Azurite was used?
Latest (v3.11.0)
Where do you get Azurite? (npm, DockerHub, NuGet, Visual Studio Code Extension)
DockerHub (
mcr.microsoft.com/azure-storage/azurite:3.11.0
)What's the Node.js version?
Whatever the
mcr.microsoft.com/azure-storage/azurite:3.11.0
docker container is runningWhat problem was encountered?
Blob.AppendBlock
gets resolved toAppendBlobHandler.create()
instead ofAppendBlobHandler.appendBlock()
. This is happening because the API specification requires thecomp
query parameter for theAppendBlob_AppendBlock
operation, while theAppendBlob_Create
operation requires thex-ms-blob-type
header, which makes it equivalent to theAppendBlob_Create
operation indispatchMiddleware
(keys 59 and 60 return the sameconditionsMet
) because both of them contain theContent-Length
header.Would it be possible to make the
x-ms-blob-type
header mandatory for theAppendBlob_AppendBlock
operation? This will probably require doing the same forAppendBlob_AppendBlockFromUrl
, so it won't end up being confused withAppendBlob_AppendBlock
. I'm happy to submit a PR for this if you can provide the steps to re-generate the code from the swagger definition.Steps to reproduce the issue?
Run the following code:
Have you found a mitigation/solution?
Sadly, no workaround...
Would it be possible to make the
x-ms-blob-type
header mandatory for theAppendBlob_AppendBlock
operation? This will probably require doing the same forAppendBlob_AppendBlockFromUrl
, so it won't end up being confused withAppendBlob_AppendBlock
. I'm happy to submit a PR for this if you can provide the steps to re-generate the code from the swagger definition.I would very much hope to see this fixed, if possible. Upgrading to the new azure-storage-blob-go API is not feasible for now (although I think that one might encounter the same issue).