Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement continuation token size limit #30600

Merged
merged 10 commits into from
Jun 8, 2023

Conversation

bambriz
Copy link
Member

@bambriz bambriz commented Jun 1, 2023

Description

This PR adds continuation token size limit support to the python sdk. The new option for item queries accepts a non-negative integer value and is represented in terms of KB, ie 2 is 2kb or 2048 Bytes. This PR fixes issue #27625

sample:
query_iterable = container.query_items(query="SELECT * FROM c where c.some_field = 'some_value'", max_item_count=max_count, partition_key=Some_Partition_Key, response_continuation_token_limit_in_kb=8)

This query will limit the continuation token size to 8KB.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

this adds continuation token size limit tot he python sdk. The new option for item queries accepts an integer value greater than 1 and is represented in terms of KB, ie 2 is 2kb or 2048 Bytes.
@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-cosmos

Copy link
Member

@simorenoh simorenoh left a comment

Choose a reason for hiding this comment

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

Couple small comments here and there, implementation looks good overall though.

Also missing an update to the Changelog and the addition of samples to show users how the option can be used. Please add those and ask for another review, thanks!

.gitignore Outdated Show resolved Hide resolved
sdk/cosmos/azure-cosmos/azure/cosmos/aio/_container.py Outdated Show resolved Hide resolved
added samples for using the new feature of limiting continuation token size.
@simorenoh
Copy link
Member

Seems like the pipelines are complaining but otherwise LGTM! Let's try to get other reviews so we can release this after the next stable release.

This should prevent CSpell error for necessary string literal.
Ignore line too long
Added provisional for the new keyword we are adding with this feature.
Copy link
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.

There are no tests for this feature.
@bambriz - for every feature we add, we should aim to add tests to make sure we test it and catch future regressions.

sdk/cosmos/azure-cosmos/azure/cosmos/aio/_container.py Outdated Show resolved Hide resolved
sdk/cosmos/azure-cosmos/azure/cosmos/container.py Outdated Show resolved Hide resolved
For now this test verifies that the continuation token limit size was passed in the request headers.
sdk/cosmos/azure-cosmos/CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Kushagra Thapar <kushuthapar@gmail.com>
@bambriz bambriz merged commit cc81110 into Azure:main Jun 8, 2023
15 checks passed
bambriz added a commit that referenced this pull request Jun 8, 2023
simorenoh pushed a commit that referenced this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Async CosmosDB client raises: 'Got more than 8190 bytes (11994) when reading Header value is too long.'
4 participants