fix: PageIterator does not correctly capture the @odata.deltaLink - #1117
Open
Diederik Greveling (DPGrev) wants to merge 5 commits into
Open
fix: PageIterator does not correctly capture the @odata.deltaLink#1117Diederik Greveling (DPGrev) wants to merge 5 commits into
Diederik Greveling (DPGrev) wants to merge 5 commits into
Conversation
PageResult had no field/deserializer for @odata.deltaLink, so it was silently dropped when parsing a page response. PageIterator also read the delta link from a Parsable model via getattr(response, '@odata.deltaLink', ''), which can never match a real attribute name and always returned an empty string, and it only inspected the very first response, never updating delta_link as later pages (where the delta link normally appears) were fetched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches the .NET SDK's PageIterator, which checks AdditionalData before the strongly-typed OdataDeltaLink property. A custom constructor_callable model that doesn't declare odata_delta_link would otherwise silently lose the delta link, same failure mode as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Keeps the field addition purely additive for positional construction (PageResult(next_link, value)), avoiding a breaking change for any downstream caller not using keyword arguments, per the repo's contribution guideline against interface-breaking trivial PRs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Use PEP 604 union syntax for the type hint and extract the repeated '@odata.deltaLink' literal into a constant.
Removes boilerplate for the additional_data fallback test without losing coverage of that code path.
|
Author
|
@microsoft-github-policy-service agree |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PageIterator.delta_linkwas effectively always empty for real usage, for two separate reasons:PageResulthad no field or deserializer for@odata.deltaLink, so it was silently dropped when a page response was parsed.PageIterator.__init__read the delta link viagetattr(response, '@odata.deltaLink', ''), which is never a valid attribute name on a KiotaParsablemodel, so it always fell through to''.next()— but in a real multi-page delta sync, the delta link normally only appears on the final page.This PR:
odata_delta_linkfield (and its@odata.deltaLink(de)serialization) toPageResult.PageIteratorto read the delta link correctly and to keep it updated asnext()advances through pages.additional_datafallback when extracting the delta link, matching how the .NET SDK'sPageIteratorchecksAdditionalDatabefore its strongly-typedOdataDeltaLinkproperty — this covers a customconstructor_callablemodel that doesn't declare the field explicitly.Test plan
pytest tests/tasks/test_page_iterator.py tests/tasks/test_page_result.py— added two regression tests: a multi-page delta sync where the delta link only appears on the final page, and a page model that only carries the delta link inadditional_data.pytest tests— 76 passed.pylint src/msgraph_core/tasks/page_iterator.py src/msgraph_core/models/page_result.py— 10.00/10.