Result Pagination and REST improvements #39
Conversation
ixmp4/data/api/base.py
Outdated
|
|
||
| if self.enumeration_method == "GET": | ||
| params.update(kwargs) | ||
| def _enumeration_request( |
There was a problem hiding this comment.
I had to think twice before getting that this method actually executes the request and does not just prepare it.
At least for me _request_enumeration is easier to understand.
|
@meksor thanks for pointing out the client side code for unpacking the pagination! I had a look at this section of the code and left two comments. Since I just reviewed scse-component/chart today: @deepak-shah-np I am fairly sure this change will break how the chart component fetches data from ixmp4. So I guess a release of these changes should be coordinated with the ssp-extensions app. |
|
That was not on my radar, so thanks. |
glatterf42
force-pushed
the
feature/pagination
branch
from
70ab41d
to
246c0ae
Compare
Sounds like a good way to go forward. That will also give me the opportunity to implement paging in ixmp4-ts once you released these changes as docker image and nice to have ixmp4-ts for the future, so if scse-apps just depend on that (and not the ixmp4 REST API directly) we can incorporate changes in the ixmp4 REST API swiftly in all scse-apps. |
|
I just did some tabulation benchmarks with different default page sizes: Seems as though larger page sizes also mean longer requests, although the difference is not so noticeable and the TCP round trip time on a real production setup will probably counteract some of the effects. I've now set the default page size to 5_000 and the maximum to 10_000. I've also added these numbers as parameters to the settings object so they can be set via the env variables |
In this PR:
Removed GET list endpoints
I've removed all the GET endpoints to reduce redundancy, also all query endpoints now use the filter framework (the timeseries endpoint didn't before).
Pagination
Each query endpoint now returns all results wrapped in a standardized object:
{ "pagination": {"limit":1000, "offset": 0} "total": <total no of objects in the db for the query> "results": [...] }Where
resultscan be a list of objects or a serialized datatable.The query parameters
limitandoffsetcan be used to control pagination. Default and maximum values forlimitare yet to be found out.@pmussak
The client side code for unpacking the pagination can be found here:
https://github.com/iiasa/ixmp4/pull/39/files#diff-6b8ca4d71cf682a8211f6d8d527b684fc42f7034d03c955505f9c0a327b93ff3R148-R203
(it should also be backwards compatible)
Bug fixes and code improvements
There was quite a few bugs and definitely unintentional typos that i found while going through the rest server.
All filter arguments in the rest layer endpoints SHOULD NO LONGER set any defaults. If I wanted to change a default I would need to change it in the filter class and in the rest layer which is confusing. I already found a bug thanks to this where the default run filter included
is_default=Falsewhich is most certainly not right. Please watch out @glatterf42.As of now the run filter should set
default_only=Trueby default (this wasn't the case everywhere by default). This affects the rest endpoints and backend layer. Is this correct @danielhuppmann?Test Data Generation
will generate some test data and fill the given platform with it.
There is also a clas for programmatic use in
ixmp4.data.generator.IamcData/Repo
I've refactored
IamcDataandIamcRepositoryintoRunIamcDataandPlatformIamcDataas this seems like the better approach. (fyi @danielhuppmann)Type Hints
All type hints concerning tabulation and listing have become a more concrete list[T] or List[T] instead of the current Iterable[T].
Upload Chunking/Pagination (to solve the "timeout" problem when uploading large datasets via rest)
... is not included in this PR and will come in the next one.