[Feature Request] Add a possibility to specify partition key version for Hash partition key type

[Agazoth]

Description

SQL database container defaults to 2 if omitted. This fixes that issue

Fixes #1852 (closed by accident)

Pipeline Reference

Pipeline

Type of Change

• Update to CI Environment or utlities (Non-module effecting changes)
• Azure Verified Module updates:
  • Bugfix containing backwards compatible bug fixes, and I have NOT bumped the MAJOR or MINOR version in version.json:
    • Someone has opened a bug report issue, and I have included "Closes #{bug_report_issue_number}" in the PR description.
    • The bug was found by the module author, and no one has opened an issue to report it yet.
  • Feature update backwards compatible feature updates, and I have bumped the MINOR version in version.json.
  • Breaking changes and I have bumped the MAJOR version in version.json.
  • Update to documentation

Checklist

• I'm sure there are no other open Pull Requests for the same update/change
• I have run Set-AVMModule locally to generate the supporting module files.
• My corresponding pipelines / checks run clean and green without any errors or warnings

[bryansan-msft]

@Agazoth I have multiple remarks here:

• Version 2 is only needed for large partition keys or Hierarchical partition keys as far as I know.
• I removed support for Range partition key in feat: avm/res/document-db/database-account public network restricted access and new account level flags #1407
• The default kind for partition key is 'Hash' so the default value for partition key version will be 1. This is intentionally.
• You need to add a test for your change, which in your case you won't be able to do as Range partition key is unsupported as mentioned above.

What is the issue you are having or is there a new feature you would like to use?
Were you trying to propose to make version 2 the new default?

PD: With your proposed changed I just realized I missed to delete Range from this allowed array. However, it is not worth to release a new version just for that.

bicep-registry-modules/avm/res/document-db/database-account/sql-database/container/main.bicep

Lines 46 to 52 in a11ba13

	@description('Optional. Default to Hash. Indicates the kind of algorithm used for partitioning.')
	@allowed([
	'Hash'
	'MultiHash'
	'Range'
	])
	param kind string = 'Hash'

[Agazoth]

@bryansan-msft thanks for your feedback!

I'm currently working on converting several deployments from using local modules/another module registry to AMV modules.

One of the larger deployments have a Cosmos DB, that has been deployed with a very basic setup, and it would not accept the current AVM module, because it had version 2 for Hash.

I see now, that this setting was actually requested by the original template author and you are probably right about keeping version on 1 (I wrongly assumed 2 would be the correct default).

I do however think, that it should be possible to change the version on hash, since there are 2 options. The setting cannot be changed after the resource is deployed, and we still need to flow version 2 to the deployment.

I would be glad to add the parameter to the module, if there is a branch I can append it to or I can create a new branch and pr. I could remove Ranged in the too :-)

[bryansan-msft]

Well, then feel free to open a PR or modify this one to add an optional partition key version parameter for the UDT of SQL containers. Take into account the following:

• The default version value of "Hash" partition key kind must remain 1.
• This new parameter must be optional.
• The version value of "MultiHash" partition key kind must always be 2.
• Add tests in the SQL test suite for your changes.

[Agazoth]

Well, then feel free to open a PR or modify this one to add an optional partition key version parameter for the UDT of SQL containers. Take into account the following:

• The default version value of "Hash" partition key kind must remain 1.
• This new parameter must be optional.
• The version value of "MultiHash" partition key kind must always be 2.
• Add tests in the SQL test suite for your changes.

Done :-)

[bryansan-msft]

Well, then feel free to open a PR or modify this one to add an optional partition key version parameter for the UDT of SQL containers. Take into account the following:

• The default version value of "Hash" partition key kind must remain 1.
• This new parameter must be optional.
• The version value of "MultiHash" partition key kind must always be 2.
• Add tests in the SQL test suite for your changes.

Done :-)

I will need you to resolve the conflict you are heaving in main.json

[Agazoth]

@bryansan-msft Alexander is pushing some punctuation updates to the file on the main branch. I merged his changes into my branch. It is updated now and good to go.

[AlexanderSehr]

@bryansan-msft Alexander is pushing some punctuation updates to the file on the main branch. I merged his changes into my branch. It is updated now and good to go.

Sorry about that @Agazoth, we updated some tests as they were not correctly picking up nested properties to validate them - and with the fix we had to apply a couple small updates to a handful of modules.

[Agazoth]

@bryansan-msft Alexander is pushing some punctuation updates to the file on the main branch. I merged his changes into my branch. It is updated now and good to go.

Sorry about that @Agazoth, we updated some tests as they were not correctly picking up nested properties to validate them - and with the fix we had to apply a couple small updates to a handful of modules.

No problem @AlexanderSehr , we'll just catch the next run 😊