Skip to content

Fix: Allow tuples to be used for hierarchical partition key values.#38136

Merged
bambriz merged 15 commits intoAzure:mainfrom
bambriz:subpartitiontuple
Nov 12, 2024
Merged

Fix: Allow tuples to be used for hierarchical partition key values.#38136
bambriz merged 15 commits intoAzure:mainfrom
bambriz:subpartitiontuple

Conversation

@bambriz
Copy link
Copy Markdown
Member

@bambriz bambriz commented Oct 28, 2024

Description

This PR fixes an issue where passing in a tuple will cause the following error to be thrown: ValueError: 2 partition key components provided. Expected less than 2 components (number of container partition key definition components).

Which should only occur when doing a prefix partition query. This pr allows passing in tuples and not just lists for subpartitioning. Both tuples and lists are serialized to json arrays and json arrays are ultimately deserialized to lists by default. This also updates type hinting to show tuples can be used the same as lists for pk values.
This pr fixes: Issue 37638

Allows the usage of tuples alongside lists to be used for hierarchical partition key values. Both get serialized to json arrays. Previous behaviour would give an error that a prefix query was attempted which is incorrect behaviour.
@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Oct 28, 2024

/azp run python - cosmos - tests

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Oct 28, 2024

/azp run python - cosmos - tests

Comment thread sdk/cosmos/azure-cosmos/azure/cosmos/partition_key.py Outdated
Copy link
Copy Markdown
Member

@xinlian12 xinlian12 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

Copy link
Copy Markdown
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

I am curious, why tests are not running on this PR?

Blocking because of testing. @scbedd did we change anything?

@kushagraThapar kushagraThapar dismissed their stale review October 29, 2024 21:36

CI has been fixed.

@kushagraThapar
Copy link
Copy Markdown
Member

@bambriz can you please re-run the CI before merging this PR.

Copy link
Copy Markdown
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

Curious about couple of things.

Comment thread sdk/cosmos/azure-cosmos/azure/cosmos/aio/_container.py Outdated
Comment thread sdk/cosmos/azure-cosmos/azure/cosmos/_base.py Outdated
@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Oct 29, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Oct 30, 2024

/azp run python - cosmos - ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Copy Markdown
Collaborator

API change check

API changes are not detected in this pull request.

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Oct 30, 2024

/azp run python - cosmos - ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 1, 2024

/azp run python - cosmos - ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 1, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Comment thread sdk/cosmos/azure-cosmos/azure/cosmos/_base.py Outdated
Changes the type hint of partition key to be sequence instead of explicitly tuple and list.
@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 6, 2024

/azp run python - cosmos - ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 6, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Pull request contains merge conflicts.

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 6, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 7, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 8, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 8, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 8, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bambriz
Copy link
Copy Markdown
Member Author

bambriz commented Nov 11, 2024

/azp run python - cosmos - tests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

update version to 4.8.0
@bambriz bambriz merged commit 65edd73 into Azure:main Nov 12, 2024
l0lawrence pushed a commit to l0lawrence/azure-sdk-for-python that referenced this pull request Feb 19, 2025
…zure#38136)

* Fix support for allowing tuples to be used for subapartition

Allows the usage of tuples alongside lists to be used for hierarchical partition key values. Both get serialized to json arrays. Previous behaviour would give an error that a prefix query was attempted which is incorrect behaviour.

* update docstrings

* Update CHANGELOG.md

* Update partition_key.py

* Update partition_key.py

optimization

* Update partition_key.py

* Update partition_key.py

* Update partition_key.py

* Update type hint of partition key

Changes the type hint of partition key to be sequence instead of explicitly tuple and list.

* update version

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

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

azure.cosmos hierarchical partition key bug

6 participants