Skip to content

Add partition colocation rule#188

Open
avinashkamat48 wants to merge 1 commit into
AzureCosmosDB:mainfrom
avinashkamat48:codex/partition-colocation
Open

Add partition colocation rule#188
avinashkamat48 wants to merge 1 commit into
AzureCosmosDB:mainfrom
avinashkamat48:codex/partition-colocation

Conversation

@avinashkamat48

Copy link
Copy Markdown
Contributor

Summary

  • add a partition colocation best-practice rule
  • show why related items should share a partition key for transactional batch and single-partition reads

Verification

  • git diff --check

Closes #171

@TheovanKraay TheovanKraay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, the colocation concept is valid, but it's already covered by the existing model-type-discriminator.md rule, which describes co-locating related entity types that share a partition key and transactional boundary. partition-query-patterns.md also covers the access pattern alignment side. That said, your PR adds value by explicitly calling out TransactionalBatch usage, which the existing rule doesn't mention. Could you enhance model-type-discriminator to incorporate that angle instead of adding a new overlapping rule? Your order/order-line example would work well there, just add a type discriminator field to stay consistent.

@TheovanKraay

Copy link
Copy Markdown
Contributor

Just a follow-up: we recently merged #204, which added topic-specific skills alongside the monolith. We're in a transitional phase where both coexist while we evaluate whether agent routing can handle the split on its own. Until that's resolved, new rules need to live in both places.

Since this is a partition- prefixed rule, please also do the following when addressing the other review feedback:

  1. Copy skills/cosmosdb-best-practices/rules/partition-colocation.md to skills/cosmosdb-partition-key/rules/partition-colocation.md
  2. Run npm run build to regenerate AGENTS.md for both skills
  3. Commit the changes

@jaydestro

Copy link
Copy Markdown
Contributor

@avinashkamat48 there are some ongoing changes being evaluated to the structure that could require this to be modified. you'll definetely get notice when it's time to make any changes to avoid merge conflicts.

@TheovanKraay

Copy link
Copy Markdown
Contributor

Thanks for this, the colocation concept is valid, but it's already covered by the existing model-type-discriminator.md rule, which describes co-locating related entity types that share a partition key and transactional boundary. partition-query-patterns.md also covers the access pattern alignment side. That said, your PR adds value by explicitly calling out TransactionalBatch usage, which the existing rule doesn't mention. Could you enhance model-type-discriminator to incorporate that angle instead of adding a new overlapping rule? Your order/order-line example would work well there, just add a type discriminator field to stay consistent.

@avinashkamat48 FYI, we've backed out skill splitting so instructions on separate note not required. However, this does have overlap with existing rules. As we're trying to be as lean as possible and avoid duplication, please address above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Rule] Collocate related items in the same partition for single-partition transactions

3 participants