Skip to content

fix: guard get_next/get_previous against unset _more - #120

Open
CedricConday wants to merge 1 commit into
predicthq:masterfrom
CedricConday:fix/resultset-more-guard
Open

fix: guard get_next/get_previous against unset _more#120
CedricConday wants to merge 1 commit into
predicthq:masterfrom
CedricConday:fix/resultset-more-guard

Conversation

@CedricConday

Copy link
Copy Markdown

Description

ResultSet.get_next() / get_previous() guard with not hasattr(self, "_more"), but _more is a declared Pydantic private attribute with a default of None:

class ResultSet(BaseModel):
    _more: Optional[Callable] = None

Because the attribute is declared, hasattr(self, "_more") is always True (it resolves to the default None), so the guard never short-circuits. When _more has not been set — e.g. a ResultSet that wasn't produced by the @returns decorator — the methods fall through to self._more(**params) and raise:

TypeError: 'NoneType' object is not callable

This changes the guard to self._more is None, so get_next() / get_previous() return None as intended when there is no callable to page with. The normal decorator-driven flow (where _more is a functools.partial) is unchanged.

Testing

Added test_resultset_without_more_returns_none, which constructs a ResultSet with next/previous set but no _more and asserts both accessors return None. Before the change this test raised TypeError; the existing test_resultset (decorator-driven paging) continues to pass.

ResultSet._more is a declared Pydantic private attribute defaulting to None, so
hasattr(self, "_more") is always True and the existing guard never triggers.
When _more is not set (e.g. a ResultSet not produced by the @returns decorator),
get_next()/get_previous() called None(**params) and raised "'NoneType' object is
not callable". Guard on `self._more is None` instead so these methods return
None as intended. Adds a regression test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant