Repository navigation
pagination: Add KeyedPage interface - #3947
Merged
Merged
Conversation
github-actions
Bot
added
edit:networking
|
coverage: 63.584% (+0.002%) from 63.582% — shiftstack:keyed-pages into gophercloud:main |
Contributor
|
@stephenfin once the conflicts are solved, I believe this will be good to go. 👌 |
stephenfin
force-pushed
the
keyed-pages
branch
from
August 19, 2026 17:17
87f1e35 to
ca0b39d
Compare
Contributor
Author
Done. I also split the change into separate commits to make it clearer what I'm doing here. |
Commit 0e6c7c7 (gophercloud#3945) ensured how we retrieve the key from a response body, ensuring we don't get skip this if the page is empty. However, we still re-evaluate the key for each page rather than using the key we determined once. This is wasteful and unnecessary complexity. Fix it now, ahead of the introduction of KeyedPage. Signed-off-by: Stephen Finucane <[email protected]>
Currently, pagination of list resources works by scanning resource bodies, looking for the first non-"links" resource key. This mostly works but is crude. It also doesn't work for resources that have multiple non-"links" top-level keys, such as placement's list resource provider inventories API, which has both a 'inventories' envelope key and a separate 'resource_provider_generation' key [1]. Improve this by adding a new KeyedPage interface, with a ResourceKey() string value. When a Page struct implements this interface, the static key will be used rather than us scanning the body for the key. When this interface is *not* implemented, we continue to fallback to the old heuristic. The latter will allow us to implement the interface incrementally, and we only do it here for NetworkPage in openstack/networking/v2/networks/results.go. [1] https://docs.openstack.org/api-ref/placement/#list-resource-provider-inventories Signed-off-by: Stephen Finucane <[email protected]>
stephenfin
force-pushed
the
keyed-pages
branch
from
August 19, 2026 17:19
ca0b39d to
50e5c69
Compare
winiciusallan
approved these changes
Aug 19, 2026
mandre
reviewed
Aug 20, 2026
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
4 participants
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.
Currently, pagination of list resources works by scanning resource bodies, looking for the first non-"links" resource key. This mostly works but is crude. It also doesn't work for resources that have multiple non-"links" top-level keys, such as placement's list resource provider inventories API, which has both an
inventoriesenvelope key and a separateresource_provider_generationkey.Improve this by adding a new
KeyedPageinterface, with aResourceKey()string value. When aPagestruct implements this interface, the static key will be used rather than us scanning the body for the key. When this interface is not implemented, we continue to fallback to the old heuristic. The latter will allow us to implement the interface incrementally, and we only do it here forNetworkPageinopenstack/networking/v2/networks/results.go.