Merge pr 35022 into pr34694 - Add Start Time To CF, Fix Retryable Exception in First Page Bug, and Fix Case Sensitive Headers Bug #35090

bambriz · 2024-04-05T18:08:32Z

Description

Original PR 35022
This removes merge conflicts with PR 34694

PR 35022 had an infinite loop issue with queries. Infinite Loop was removed while keeping the fix to the retryable exception bug in the first page of a query. This PR also adds Start Time option in change feed pulls, and fixes a bug where response headers were being returned and used as Case Sensitive Dictionaries. Response Headers are now Case Insensitive Multi Dictionaries.

A utc datetime is passed into the options when calling a change feed pull. If the datetime is not utc it will be converted to utc.

The following Example creates 10 items at two different times. By using start time you can filter out the items created after the time recorded.

    batchSize = 10
    container = database.get_container_client(container_name)
    create_random_items(container, batchSize)
    sleep(1)
    now = datetime.now(timezone.utc)
    sleep(1)
    create_random_items(container, batchSize)
    change_feed_iter = container.query_items_change_feed(start_time=now)

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.

Adds start time, at the moment has a work around for getting all the documents.

Start time change feed queries always result in the initial fetch results being empty, which is a deviation in behaviour from other change feed options. To accommodate that we check if we have the start time change feed option and check if it is the first fetch request in order to grab the continuation value from etag to get the rest of the results.

Added a new header to remove ambuiguity between 'etag' meant for sync clients and 'Etag' meant for async clients in our code for header strings.

added negative test for start time option of change feed.

…ged prs Response headers are now CIMultDict type (Case insensitive Multi Dictionaries) which fixes any issues regarding casing of header strings. This also has changes to merge changes from PR 35022 without issues.

…-PR34694

This code checks if a 429 error has occured in which case we reset has started and try getting the fetch items again

sdk/cosmos/azure-cosmos/azure/cosmos/_execution_context/aio/base_execution_context.py

azure-sdk · 2024-04-05T18:36:06Z

API change check

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

azure-cosmos

Added code to help debug. This will catch 429 exception when it occurs on first page of query so we can see whats going on.

Moving the has_Started flag to after the fetch function seems to make the behaviour better align with what is expected. Retries are properly done for queries that got 429 in the first page of their query, and 429s are also being properly surfaced when retries runout.

Remove trailing spaces, bad indentation, and unused imports

bambriz · 2024-04-08T18:12:23Z

/azp run python - cosmos - tests

azure-pipelines · 2024-04-08T18:12:38Z

Azure Pipelines successfully started running 1 pipeline(s).

sdk/cosmos/azure-cosmos/azure/cosmos/_base.py

simorenoh · 2024-04-08T23:00:14Z

/azp run python - cosmos - tests

azure-pipelines · 2024-04-08T23:01:06Z

Azure Pipelines successfully started running 1 pipeline(s).

This push adds test coverage for the retryable exception bug that this PR fixes. It recreates the scenario of the original bug and verifies that the fix is working. This also changes the start time datetime to always be converted to UTC regardless of timezone or if it is a naive datetime.

bambriz · 2024-04-10T18:02:36Z

/azp run python - cosmos - tests

azure-pipelines · 2024-04-10T18:02:52Z

Azure Pipelines successfully started running 1 pipeline(s).

simorenoh

awesome work Bryan, thanks!

Fixed outdated comments to better match new code.

kushagraThapar

Great work @bambriz, thanks for fixing the other bugs as well. Added some comments, and requesting changes for samples on start time usage in change feed query.

sdk/cosmos/azure-cosmos/CHANGELOG.md

sdk/cosmos/azure-cosmos/azure/cosmos/_execution_context/aio/base_execution_context.py

sdk/cosmos/azure-cosmos/azure/cosmos/_execution_context/base_execution_context.py

sdk/cosmos/azure-cosmos/azure/cosmos/aio/_container.py

sdk/cosmos/azure-cosmos/azure/cosmos/container.py

sdk/cosmos/azure-cosmos/azure/cosmos/_base.py

Co-authored-by: Kushagra Thapar <kushuthapar@gmail.com>

Added further optimizations.

bambriz · 2024-04-10T22:10:50Z

/azp run python - cosmos - tests

azure-pipelines · 2024-04-10T22:11:06Z

Azure Pipelines successfully started running 1 pipeline(s).

pylint fixes

bambriz · 2024-04-10T22:47:31Z

/azp run python - cosmos - tests

azure-pipelines · 2024-04-10T22:47:46Z

Azure Pipelines successfully started running 1 pipeline(s).

kushagraThapar

LGTM, thanks @bambriz
Please also add the sample for using start from time functionality :)

Add samples for start time change feed

bambriz · 2024-04-11T16:59:22Z

/azp run python - cosmos - tests

azure-pipelines · 2024-04-11T16:59:38Z

Azure Pipelines successfully started running 1 pipeline(s).

annatisch · 2024-04-18T20:59:45Z

sdk/cosmos/azure-cosmos/azure/cosmos/container.py

@@ -342,6 +345,8 @@ def query_items_change_feed(
            feed_options["partitionKey"] = self._set_partition_key(partition_key)
        if is_start_from_beginning is not None:
            feed_options["isStartFromBeginning"] = is_start_from_beginning
+        if start_time is not None and is_start_from_beginning is False and isinstance(start_time, datetime):


This check isn't correct - we should not be doing an isinstance check for start_time - because this will fail silently if they pass in something else. We should just let it fail if they pass in the wrong type.

bambriz and others added 13 commits March 7, 2024 13:34

Add start time to change feed query

a593860

Adds start time, at the moment has a work around for getting all the documents.

Merge remote-tracking branch 'upstream/main' into read-specific-time

ca59d5e

Add new HTTP Header for Async Etag

79cd79b

Added a new header to remove ambuiguity between 'etag' meant for sync clients and 'Etag' meant for async clients in our code for header strings.

Update start time tests

c07dbfc

added negative test for start time option of change feed.

comment fix

34d06d4

Readme and changelog updates

08c3f59

Fix bug with retryable exception in first page

995189e

Merge remote-tracking branch 'upstream/main' into read-specific-time

7c36ee0

Merge PR 35022

0f800b0

Merge remote-tracking branch 'upstream/main' into Merge-PR-35022-into…

d89d904

…-PR34694

Testing retry on execution context bug

5ced092

This code checks if a 429 error has occured in which case we reset has started and try getting the fetch items again

github-actions bot added the Cosmos label Apr 5, 2024

bambriz commented Apr 5, 2024

View reviewed changes

sdk/cosmos/azure-cosmos/azure/cosmos/_execution_context/aio/base_execution_context.py Show resolved Hide resolved

bambriz commented Apr 5, 2024

View reviewed changes

sdk/cosmos/azure-cosmos/azure/cosmos/_execution_context/aio/base_execution_context.py Outdated Show resolved Hide resolved

bambriz added 4 commits April 5, 2024 16:13

Debugging additions

a019b35

Added code to help debug. This will catch 429 exception when it occurs on first page of query so we can see whats going on.

update _base.py

075551f

Pylint fixes

320a8b8

Remove trailing spaces, bad indentation, and unused imports

update start time tests

9b4fd26

bambriz changed the title ~~Merge pr 35022 into pr34694~~ Merge pr 35022 into pr34694 - Add Start Time To CF, Fix Retryable Exception in First Page Bug, and Fix Case Sensitive Headers Bug Apr 8, 2024

Removed testing artifacts updated Changelog

5e4d328

bambriz marked this pull request as ready for review April 8, 2024 20:37

bambriz requested review from kushagraThapar, simorenoh and xinlian12 as code owners April 8, 2024 20:37

bambriz requested a review from annatisch as a code owner April 8, 2024 20:37

nickcoai approved these changes Apr 8, 2024

View reviewed changes

sdk/cosmos/azure-cosmos/azure/cosmos/_base.py Outdated Show resolved Hide resolved

simorenoh approved these changes Apr 8, 2024

View reviewed changes

bambriz mentioned this pull request Apr 10, 2024

Add start time to change feed query #34694

Closed

6 tasks

simorenoh approved these changes Apr 10, 2024

View reviewed changes

Fixed Comments

6b362c9

Fixed outdated comments to better match new code.

kushagraThapar requested changes Apr 10, 2024

View reviewed changes

bambriz and others added 4 commits April 10, 2024 14:49

Update sdk/cosmos/azure-cosmos/CHANGELOG.md

21f9c69

Co-authored-by: Kushagra Thapar <kushuthapar@gmail.com>

Update sdk/cosmos/azure-cosmos/CHANGELOG.md

ce6ab05

Co-authored-by: Kushagra Thapar <kushuthapar@gmail.com>

Update sdk/cosmos/azure-cosmos/CHANGELOG.md

41455b6

Co-authored-by: Kushagra Thapar <kushuthapar@gmail.com>

optimization changes

a440fde

Added further optimizations.

update _base.py

1512fa2

pylint fixes

kushagraThapar approved these changes Apr 11, 2024

View reviewed changes

Add samples for start time change feed

1aa7b1d

Add samples for start time change feed

bambriz merged commit 586f24d into Azure:main Apr 11, 2024
24 checks passed

bambriz mentioned this pull request Apr 11, 2024

when retryable error happens, cosmos client returns empty result for pagination query #35021

Closed

kushagraThapar mentioned this pull request Apr 11, 2024

Fix bug with retryable exception in first page of query #35022

Closed

5 tasks

simorenoh mentioned this pull request Apr 17, 2024

Azure SDK Review - [Azure SDK - Cosmos DB Python SDK] Azure/azure-sdk#7470

Closed

annatisch reviewed Apr 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge pr 35022 into pr34694 - Add Start Time To CF, Fix Retryable Exception in First Page Bug, and Fix Case Sensitive Headers Bug #35090

Merge pr 35022 into pr34694 - Add Start Time To CF, Fix Retryable Exception in First Page Bug, and Fix Case Sensitive Headers Bug #35090

bambriz commented Apr 5, 2024 •

edited

Loading

azure-sdk commented Apr 5, 2024

bambriz commented Apr 8, 2024

azure-pipelines bot commented Apr 8, 2024

simorenoh commented Apr 8, 2024

azure-pipelines bot commented Apr 8, 2024

bambriz commented Apr 10, 2024

azure-pipelines bot commented Apr 10, 2024

simorenoh left a comment

kushagraThapar left a comment

bambriz commented Apr 10, 2024

azure-pipelines bot commented Apr 10, 2024

bambriz commented Apr 10, 2024

azure-pipelines bot commented Apr 10, 2024

kushagraThapar left a comment

bambriz commented Apr 11, 2024

azure-pipelines bot commented Apr 11, 2024

annatisch Apr 18, 2024

Merge pr 35022 into pr34694 - Add Start Time To CF, Fix Retryable Exception in First Page Bug, and Fix Case Sensitive Headers Bug #35090

Merge pr 35022 into pr34694 - Add Start Time To CF, Fix Retryable Exception in First Page Bug, and Fix Case Sensitive Headers Bug #35090

Conversation

bambriz commented Apr 5, 2024 • edited Loading

Description

All SDK Contribution checklist:

General Guidelines and Best Practices

Testing Guidelines

azure-sdk commented Apr 5, 2024

bambriz commented Apr 8, 2024

azure-pipelines bot commented Apr 8, 2024

simorenoh commented Apr 8, 2024

azure-pipelines bot commented Apr 8, 2024

bambriz commented Apr 10, 2024

azure-pipelines bot commented Apr 10, 2024

simorenoh left a comment

Choose a reason for hiding this comment

kushagraThapar left a comment

Choose a reason for hiding this comment

bambriz commented Apr 10, 2024

azure-pipelines bot commented Apr 10, 2024

bambriz commented Apr 10, 2024

azure-pipelines bot commented Apr 10, 2024

kushagraThapar left a comment

Choose a reason for hiding this comment

bambriz commented Apr 11, 2024

azure-pipelines bot commented Apr 11, 2024

annatisch Apr 18, 2024

Choose a reason for hiding this comment

bambriz commented Apr 5, 2024 •

edited

Loading