search

Azure / ms-rest-js

Runtime for isomorphic javascript libraries generated by Autorest
MIT License
55 stars 55 forks source link

Adding MERGE HTTP method #400

Open iamNoah1 opened 4 years ago

iamNoah1 commented 4 years ago

Is your feature request related to a problem? Please describe. Working on a project that uses ms-rest-js and need to handle HTTP MERGE request method

Describe the solution you'd like Add MERGE as possible request method

xirzec commented 4 years ago

Looks like this needs to get added to https://github.com/Azure/ms-rest-js/blob/01cfc78e2fe58d376cf1398f758d6e34415864c8/lib/webResource.ts#L12

Can you add some context about which service is using this?

iamNoah1 commented 4 years ago

I needed that for Azurite table, but in the meanwhile the other contributers added a solution for solving my blocker (https://github.com/Azure/Azurite/pull/545/files). But TBH I cannot say if that is only a workaround. Anyways I think adding MERGE method is relevant anyways.

xirzec commented 4 years ago

Looking at this again, it seems like MERGE is not a standard request method: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods

And this page makes it sound like PATCH is preferred: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-odata/59d5abd3-7b12-490a-a0e2-9d9324b91893

Since MERGE is not one of the verbs that is defined in the HTTP specification [RFC2616], using the MERGE verb might not flow through network intermediaries as seamlessly as methods that are defined in the HTTP specification. The HTTP PATCH verb is preferred over HTTP MERGE when working with data services that support the OData 3.0 protocol.

I'm a little confused why storage-tables would require the use of a non-standard request method.

iamNoah1 commented 4 years ago

Interesting point. Even though MERGE seems not be a standard request method, the storage api specifies MERGE as method for the merge entity operation: https://docs.microsoft.com/en-us/rest/api/storageservices/merge-entity. I think PATCH is also valid, but Azurite wants to be API spec compliant to the storage API spec.

xirzec commented 4 years ago

Technically I think you could set the method with "MERGE" as any and it would work, if you just want TS not to complain.

jeremymeng commented 3 years ago

@iamNoah1 would the workaround of "MERGE" as any work for you?