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
154 stars 46 forks source link

Validate PartitionKey when Calling *-CosmosDbDocument/Attachment Functions #275

Open PlagueHO opened 5 years ago

PlagueHO commented 5 years ago

Issue

Calling one of the -CosmosDbDocument or -CosmosDbAttachment functions against a collection with a custom partition key requires that the partition key is specified, otherwise a http 400 error is returned.

It would be good if a pre-validation check would occur before making the REST call to alert the user of the condition. The check should be able to be suppressed with a -SkipPartitionKeyCheck parameter to enable better performance and reduced cost.

This needs to be considered carefully though because it will reduce performance and increase cost if automation is not updated to use the -SkipPartitionKeyCheck. It might be preferrable to detect the condition when the 400 error is received and display the information clearly to the user.

tamusjroyce commented 4 years ago

This is similar to issues I was having. In the Full Sample I added to https://github.com/PlagueHO/CosmosDB/issues/382, I manually added the partitionKey if it is missing in the document being saved. Making 404 for missing partitionKey impossible.

Would it be a performance hit if New-CosmosDbDocument took in a parameter for collection context returned from Get-CosmosDbCollection instead of -CollectionId $containerName? Then -PartitionKey $partitonValue could be omitted (calculated inside New-CosmosDbDocument)

By New-CosmosDbDocument, I mean -CosmosDbDocument or -CosmosDbAttachment too

With that, I don't know if -SkipPartitionKeyCheck would be needed. If you pass in -CollectionConext $collectionContext, the partition check will occur. If you use -CollectionId $containerName, it will not.

Containers not having a partition now have /_partitionKey. Once this key is populated, it acts as a partition key for that document. Once all documents have _partitionKey, it sounds like the collection is effectively a partitioned container. https://docs.microsoft.com/en-us/azure/cosmos-db/migrate-containers-partitioned-to-nonpartitioned

So if non-partitioned collections have _partitionKey, why not treat them like partitioned collections and ignore the fact that they are non-partitioned? Would that cause any exceptions? Would sending _partitionKey value as $null cause an exception?

Just hoping that if New-CosmosDbDocument uses -CollectionConext $collectionContext that no extra code needs implemented to handle non-partitioned collections. It would be fine to use '_partitionKey' and stop omitting it.

tamusjroyce commented 4 years ago

Let me know if this is a good idea and I will put together a PR (might be a few weeks)

PlagueHO commented 4 years ago

Hi @tamusjroyce - that is a good suggestion. Let me think about this for a little bit.

My gut feeling is that this should be fine as long as CollectionContext was optional. Rather than calling the parameter CollectionContext it should be Collection and should also be a pipeline (match on type) parameter so that you could do this:

Get-CosmosDbCollection -Context $context -CollectionId 'Whatever' | Get-CosmosDbDocument -Content $context -Id 'abc' -PartitionKey 'abc'

This would relate it to issue #210 which I had been planning for sometime but didn't get around to it.

I'm not so worried about implementing this for Artefacts as these are essentially deprecated anyway and shouldn't be used.