Conversation
…ts, updates to checkNameAvailabilityResult
|
Sorry - missed this one bit. |
|
@anuchandy @milismsft Could one of you take a quick look? |
anuchandy
left a comment
anuchandy
left a comment
There was a problem hiding this comment.
@tiffanyachen since previous PR was squashed and merged, you need to create a new branch from upstream kv2018, add network acl feature, and open a PR from that branch to upstream kv2018
| * @return the networkAcls value | ||
| */ | ||
| @Beta(SinceVersion.V1_11_0) | ||
| NetworkRuleSet networkAcls(); |
There was a problem hiding this comment.
@tiffanyachen In Fluent we try to avoid using acronyms especially when these are not needed. In this the method can be replaced with "networkRuleSet()". It's also consistent with a similar class found in Storage management library.
| * @param networkAcls the networkAcls value to set | ||
| * @return the next stage of key vault definition | ||
| */ | ||
| WithCreate withNetworkAcls(NetworkRuleSet networkAcls); |
There was a problem hiding this comment.
@tiffanyachen Please rename this to "withNetworkRuleSet()". Also in Java we try to avoid passing a whole complex object that needs to be built aside and instead we "decompose" them in place into a sequence of "with...()" methods corresponding to each member of the class that can be set. Think of it similar to setting the AccessPolicy.
There was a problem hiding this comment.
It seems the key vault NetworkRuleSet is EXACTLY same as storage account network rule set. Customer will have consistent experience If we can ensure key vault nw ruleset experience is same as that storage provide.
Storage:
KeyVault:
Take a look at this “stage” in storage account definition
It seems you can copy over the helper, stages and implementations from storage. But double check the logic work for key vault as well.
| * @param networkAcls the networkAcls value to set | ||
| * @return the next stage of key vault definition | ||
| */ | ||
| Update withNetworkAcls(NetworkRuleSet networkAcls); |
|
Move to here #449 |
3 participants
Added networkAcl updates