Added a Capacity param on New-CosmosDbAccount - Fixes #439

[chrisjantzen]

Pull Request

Improvements / Enhancements

• Add a new Capacity param to New-CosmosDbAccount which allows you to create Cosmos DB Accounts that are Serverless or that use Cassandra, Gremlin, etc.

Closes #439

---

This change is 

[PlagueHO]

Awesome @chrisjantzen - I'll review tonight and get merged.

[PlagueHO]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @chrisjantzen)

---

docs/New-CosmosDbAccount.md, line 19 at r1 (raw file):

New-CosmosDbAccount [-Name] <String> [-ResourceGroupName] <String> [-Location] <String>
 [[-LocationRead] <String[]>] [[-DefaultConsistencyLevel] <String>] [[-MaxIntervalInSeconds] <Int32>]
 [[-MaxStalenessPrefix] <Int32>] [[-IpRangeFilter] <String[]>] [[-Capability] <String>] [[-AllowedOrgin] <String[]>]

Should Capability be an array?

---

source/Public/accounts/New-CosmosDbAccount.ps1, line 52 at r1 (raw file):

        [Parameter()]
        [ValidateSet('EnableCassandra', 'EnableTable', 'EnableGremlin', 'EnableServerless')]
        [System.String]

I'm thinking this should be an array of strings, because it looks like this parameter supports multiple Capabilties assigned to the same database (e.g. EnableTable and EnableServerless could be used in combination):
https://docs.microsoft.com/en-us/azure/templates/microsoft.documentdb/databaseaccounts?tabs=json#databaseaccountcreateupdateproperties

[chrisjantzen]

I'm thinking this should be an array of strings, because it looks like this parameter supports multiple Capabilties assigned to the same database (e.g. EnableTable and EnableServerless could be used in combination):

I looked into this further and you are absolutely correct. EnableServerless can be used in tandem with any of the other values. I have modified the Capability param to accept an array now and updated the tests to match. None of other values can be used more than once on an account and I haven't added in checks for that as the API itself will error out if you use multiple. However I have updated the documentation to note this.

Thanks!

[PlagueHO]

Cool - I think it is OK to let the Cosmos DB REST API deal with any unsupported capability combinations for now. Could add checks at this side later on if it is a problem.

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @chrisjantzen)

a discussion (no related file):
Awesome stuff @chrisjantzen - just one minor tweak and I'll merge once the build passes.

---

---

docs/New-CosmosDbAccount.md, line 19 at r2 (raw file):

New-CosmosDbAccount [-Name] <String> [-ResourceGroupName] <String> [-Location] <String>
 [[-LocationRead] <String[]>] [[-DefaultConsistencyLevel] <String>] [[-MaxIntervalInSeconds] <Int32>]
 [[-MaxStalenessPrefix] <Int32>] [[-IpRangeFilter] <String[]>] [[-Capability] <String>] [[-AllowedOrgin] <String[]>]

Can you just tweak this to show the capability as an array? E.g.:
[[-Capability] <String[]>]

[PlagueHO]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[PlagueHO]

I think this line might need to be:

capabilities = @(
  @{
    name = $script:testCapability[0]
  },
  @{
    name = $script:testCapability[1]
  }
)

[PlagueHO]

I think the tests are failing here on PS 6/7 because the output of ConvertTo-Json is non-determinate. I've run into this before - essentially the properties will not be output in the same order. So you'd need to actually compare object properties (which is a pain).

[chrisjantzen]

Thanks Daniel for your assistance! It looks like $script:mockGetAzResource is not being used in the test I created which is why the issue on line 80 never caused a problem. I copied the AllowedOrigin test above it as reference. Not sure if I've missed something though or if another test needs to be created. I am not too familiar with automated testing and have never used Pester before, so this part was new to me.

I spent quite a while debugging my tests on Tuesday; ironically my main issue in the tests was that I had swapped the order of 'EnableServerless' and 'EnableCassandra' in the mock and the actual test, so the order never matched. Of course the simple issue was the last thing I noticed. I have now modified the test to explicitly check the properties, like you did with a number of the other checks. I ran multiple tests with good and bad cases to verify this is working and it seems to work. Although I did the same yesterday and it seemed good then as well. Perhaps ConvertTo-Json is more likely to keep the order on certain systems or perhaps I just got lucky. Hopefully the tests work out this time!

[PlagueHO]

RE the ConvertTo-Json issue: This actually flows all the way back to the JSON.NET library that is used by PS. I tracked it all the way back there and raised an issue but it wasn't going to be fixed because there isn't supposed to be a garantee of the order. I've been bitten by it many times 😢

Anyway, thanks for this and I'll merge now. A preview version will be published to PS Gallery and then I'll do a full release a few days after.

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @chrisjantzen)