:recycle: [#81] Replace get_paginated_results with pagination_helper

[stevenbal]

fixes: #81

[codecov[bot]]

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (4a30f32) 72.33% compared to head (f9ac7f0) 74.09%.

Files	Patch %	Lines
zgw_consumers/legacy/service.py	77.19%	8 Missing and 5 partials :warning:
zgw_consumers/api_models/base.py	0.00%	0 Missing and 2 partials :warning:
zgw_consumers/service.py	90.90%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #82 +/- ## ========================================== + Coverage 72.33% 74.09% +1.75% ========================================== Files 39 40 +1 Lines 1522 1579 +57 Branches 216 225 +9 ========================================== + Hits 1101 1170 +69 + Misses 354 338 -16 - Partials 67 71 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[stevenbal]

@sergei-maertens I'm not entirely sure how to proceed with this, in Open Inwoner I extended the ape_pie client to perform the status checks and JSON parsing, because I feel like this is something we want to do for all requests (https://github.com/maykinmedia/open-inwoner/blob/develop/src/open_inwoner/utils/api.py#L73). Is this a pattern that makes sense? And would we want to support both clients that return the response object and the parsed response data with pagination_helper?

EDIT: I guess to keep things in line with requests.Session (which is what ape_pie is extending), it would be better to move the JSON parsing / status checking to some util or other method, that way we can keep pagination_helper the way it is

[sergei-maertens]

@stevenbal I found that this pattern got in the way quite a bit when dealing with non-JSON responses. Often other API vendors (and even Open Zaak) return HTML for server errors (HTTP 5XX) and then the error handling crashes because it can't parse it as JSON. Another example is downloading (preferably using streaming mode) the file of an informatieobject from the Documenten API, which also requires you to jump through hoops. In other cases, you may even want to process the response in caller code directly or have some robustness.

My preferred approach is building a client with semantic methods, e.g.:

class Zaak(TypedDict):
    url: str
    zaaktype: str

class ZakenClient(APIClient):

    def list_zaken(self) -> List[Zaak]:
        response = self.get("zaken")
        response.raise_for_status()
        return response.json()

This nicely encapsulates the technical implementation while exposing a nicer Python API without having to worry about the specifics of the API - which encourages loose coupling and can be useful in tests for mocking or dependency injection. In this example, you could use TypedDict to benefit from editor auto complete and type checking at the Python level, instead of operating on "arbitrary" JSON-ish data. Even more powerful (and something I want to explore!) would be to use Pydantic for a really native Python experience + run-time validation!

So I think what you're doing in Open Inwoner is an anti-pattern - the client HTTP methods (get, post... etc) should be predictable and thus conform to the requests public API. Linters/editors (like Pyright) will also warn that the overridden request method has a different method signature and violates the Liskov substitution principle.