Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Fix bug in index_resources.get_all() #365

Merged
merged 1 commit into from
Jul 24, 2018

Conversation

eamonnotoole
Copy link
Contributor

@eamonnotoole eamonnotoole commented Feb 16, 2018

Description

The essential problem is that index_resources.get_all() calls the
Resource get() method which doesn't handle pagination and thus only
returns the first page of results. The complication is that
index_resources.get_all() builds its own URI complete with filtering, so
the Resource get_all() is probably not appropriate since it builds the
complete URI from a set of provided filters. To fix we've added a new
method to Resource - get_all_prebuilt_uri - which passes the pre-built
URI directly to the underlying __do_requests_to_getall method. The
implicated unit-tests have been updated.

Issues Resolved

[Fixes #364]

Check List

  • New functionality includes testing.
    • All tests pass for Python 2.7+ & 3.4+($ tox).
  • New functionality has been documented in the README if applicable.
    • New functionality has been thoroughly documented in the examples (please include helpful comments).
    • New endpoints supported are updated in the endpoints-support.md file.
  • Changes are documented in the CHANGELOG.

@eamonnotoole
Copy link
Contributor Author

Coverage decreased because I've removed a few lines from index_resources.py

The essential problem is that index_resources.get_all() calls the
Resource get() method which doesn't handle pagination and thus only
returns the first page of results.  The complication is that
index_resources.get_all() builds its own URI complete with filtering.
Thus we call index_resources.get_all() with 'start', 'count' and 'uri'
arguments and remove the 'start' and 'count' fields from the pre-built
uri. The implicated index_resource unit-tests have been updated.

Fixes HewlettPackard#364
Copy link
Member

@soodpr soodpr left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@soodpr soodpr merged commit 418c500 into HewlettPackard:master Jul 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in index_resources.get_all()
3 participants