Azure-sdk-for-js: [Storage] MatchCondition instead of various access condition

Created on 8 Oct 2019  路  12Comments  路  Source: Azure/azure-sdk-for-js

Related discussion at https://github.com/Azure/azure-sdk/issues/638

Looks we are going to have some flags

ifChanged/ ifUnchanged

Client Storage blocking-release

Most helpful comment

I believe the intention of the guidelines is that the condition option with the onlyIfChanged set of options are simple options when an entity is passed in with an etag. For storage, this is not the case, so a more general purpose option type is needed. For that type, I think using the standard etag names as storage is already using (ifMatch/ifNoneMatch) is appropriate, and their values are strings.

One thing I'm not sure about: do we still call the opting conditions but its type is a ModifiedAccessConditions rather than the SimpleConditions with boolean flags? /cc @tg-msft in case you know what .net's plans are here :)

All 12 comments

This is only for etag

Reach out to @richardpark-msft

I think the difference between how we're doing in appconfig and how the rest of the world is doing it is that since our model object (ConfigurationSetting) is always passed as an argument we can take the etag value directly from it rather than requiring the user to pass it separately.

You can see that here they can just pass in the setting:
https://github.com/Azure/azure-sdk-for-js/blob/f67580af2d9d6255146a7f9e0c2535c30f16ec77/sdk/appconfiguration/app-configuration/samples/optimisticConcurrencyViaEtag.ts#L80

Then to "activate" it they just specify a boolean parameter:

await client.setConfigurationSetting(actualSetting, {
onlyIfUnchanged: true
});

In the etag discussion yesterday I believe that Ted mentioned that storage couldn't necessarily do that because the object that gets passed in doesn't have an etag field.

So I'm not sure if what I did would apply to what you're doing in storage.

For completeness here's the section in the design guide that talks about conditional requests:
https://github.com/Azure/azure-sdk/blob/master/docs/general/design.md#conditional-requests

I believe the intention of the guidelines is that the condition option with the onlyIfChanged set of options are simple options when an entity is passed in with an etag. For storage, this is not the case, so a more general purpose option type is needed. For that type, I think using the standard etag names as storage is already using (ifMatch/ifNoneMatch) is appropriate, and their values are strings.

One thing I'm not sure about: do we still call the opting conditions but its type is a ModifiedAccessConditions rather than the SimpleConditions with boolean flags? /cc @tg-msft in case you know what .net's plans are here :)

https://github.com/Azure/azure-sdk-for-net/pull/8138 to cross reference what we're doing in .NET

@jeremymeng, Looks like storage JS doesn't require any changes. If so, can we close this issue?

We are pretty close to .NET has currently except names. At this point I'd want to keep the old names.

Chatted with @annelo-msft - AppConfig in .net is NOT exposing MatchConditions (or it's cousin RequestConditions) for AppConfig. It's all done via the bool onlyIfChanged parameter.

I think no change is required here, so closing.

I am possibly wrong. Discussing more tomorrow.

Blob: #5672
File: #5682

Closing as #5672 and #5682 are complete

Was this page helpful?
0 / 5 - 0 ratings