search

PlagueHO / CosmosDB

PowerShell Module for working with Azure Cosmos DB databases, collections, documents, attachments, offers, users, permissions, triggers, stored procedures and user defined functions.
http://dscottraynsford.com
MIT License
152 stars 46 forks source link

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

Closed chrisjantzen closed 2 years ago

chrisjantzen commented 2 years ago

Pull Request

Improvements / Enhancements

Closes #439

This change is Reviewable

PlagueHO commented 2 years ago

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

chrisjantzen commented 2 years ago

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 commented 2 years ago

/AzurePipelines run

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 1 pipeline(s).
chrisjantzen commented 2 years ago

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!