Preliminary pagination support API #61

yakky · 2017-12-30T13:03:19Z

This implements support for pagination
Fix #59

By default list method retrieves all the objects navigating the pages returned by the API
all argument allows to disable pagination and return all the results (implementing current behavior)
lazy_pagination argument allows to load a single page

erikw · 2017-12-30T13:11:34Z

docs/index.rst

@@ -169,6 +169,27 @@ You can also specify filters

    tasks = api.tasks.list(project=1)

+By defaults list returns all objects, eventually getting the


s/By defaults/By default,/

Thanks, fixed

erikw · 2017-12-30T13:22:36Z

docs/index.rst

+
+.. code:: python
+
+   tasks_page_1 = api.tasks.list(lazy_pagination)  # Will only return page 1


Add example to get everything

a plain api.tasks.list() will still return all the results, like it does now, but using the pagination to get all the results

yakky · 2017-12-30T15:06:41Z

@erikw thanks for your feedback. Does it look a sane API to you?

yakky · 2017-12-30T15:17:18Z

@erikw I added the page_size argument to allow improved control over the page size (and hence performance)

erikw · 2017-12-30T17:46:40Z

@yakky I was thinking it was a bit confusing with the different combinations you can make with all and lazy_pagination, but it's needed if we want to support all use cases I guess.

Maybe a matrix in the documentation like this one could help?

all	lazy_pagination	Output
False	False	...
False	True	...
True	False	...
True	True	...

or possible reduce the number of use cases to support to maybe only have one input variable (with page number var too) paginated=True|False?

erikw · 2017-12-30T17:47:11Z

@yakky Looks good! I was thinking it was a bit confusing though with the different combinations you can make with all and lazy_pagination, but it's needed if we want to support all use cases I guess.

Maybe a matrix in the documentation like this one could help?

all	lazy_pagination	Output
False	False	...
False	True	...
True	False	...
True	True	...

or possible reduce the number of use cases to support to maybe only have one input variable (with page number var too) paginated=True|False?

yakky · 2017-12-30T18:20:50Z

@erikw thanks for your feedback.

We can use a 3-state argument (let's call it pagination), in addition to page argument to retrieve a specific page but I am worried it might be confusing:

`pagination`	`page`	Output
`True` (default)	`None`	All results retrieved by using paginated results and loading them behind the scenes
`True` (default)	`<integer>`	Only results for the given page are retrieved
`False`	`unused`	Current behavior: all results, ignoring pagination
`<integer>`	`None`	All results retrieved by using paginated results and loading them behind the scenes, using given page size (higher page size could yield better performances)
`<integer>`	`<integer>`	Only results for the given page of the given size are retrieved

erikw · 2017-12-30T18:23:29Z

@yakky I agree, mixing types even for the same variable is not good. As long as it's clear or alternatively documented what will happen for which values of the argument, it's all good :)

yakky · 2017-12-30T18:27:41Z

@erikw how about this signature (basically merges all and lazy_pagination)

`pagination`	`page_size`	`page`	Output
`True` (default)	`<integer>` (default: 100; non numeric values fallbacks to the default)	`None`	All results retrieved by using paginated results and loading them behind the scenes, using given page size (higher page size could yield better performances)
`True` (default)	`<integer>` (default: 100)	`<integer>`	Only results for the given page of the given size are retrieved
`False`	`unused`	`unused`	Current behavior: all results, ignoring pagination

yakky · 2017-12-30T20:19:33Z

@fmarco could you also give a look at this plz?

erikw · 2017-12-30T21:06:23Z

@yakky this last one looks sounder to me!

yakky · 2017-12-30T21:08:43Z

Thanks @erikw we will complete this with some tests in the next days and we'll likely release a new version sometimes next week

carloratm · 2018-01-05T13:56:53Z

docs/index.rst

+
+   tasks = api.tasks.list(paginate=False)
+
+.. warning:: be aware that if the unpaginated results may exceed


Remove if?

carloratm · 2018-01-05T13:57:33Z

taiga/models/base.py

+        remote objects are retrieved and appended in a single list.
+
+        If pagination is disabled, all the objects are fetched from the
+        endpojnt and returned. This may trigger some parsing error if the


fmarco

@yakky except for @carloratm's comments, lgtm

yakky · 2018-01-05T15:07:29Z

Thanks y'all. I'll fix those and I'll implement the tests for this. Hopefully we will be able to merge this and release a new version

yakky · 2018-01-07T13:06:27Z

@carloratm @fmarco could you have a look at the tests?

codecov · 2018-01-07T13:10:46Z

Codecov Report

❗ No coverage uploaded for pull request base (master@012fb19). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master      #61   +/-   ##
=========================================
  Coverage          ?   99.21%           
=========================================
  Files             ?        8           
  Lines             ?      896           
  Branches          ?        0           
=========================================
  Hits              ?      889           
  Misses            ?        7           
  Partials          ?        0

Impacted Files	Coverage Δ
taiga/requestmaker.py	`98.33% <100%> (ø)`
taiga/models/base.py	`99.28% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 012fb19...499534f. Read the comment docs.

erikw · 2018-01-10T18:36:09Z

Awesome!

When can we expect a new release of python-taiga that includes this?

yakky · 2018-01-10T18:36:58Z

@erikw with a bit of luck within the weekend 🤞

erikw · 2018-01-28T15:32:31Z

@yakky or maybe this weekend? :) 🙏

yakky · 2018-01-28T15:35:18Z

@erikw early next week 🙇 (but 0.9rc1 is already on PyPi)
0.9 will likely be identical

yakky · 2018-01-30T23:57:10Z

@erikw 0.9.0 released!

erikw · 2018-02-02T21:48:58Z

@yakky Many thanks!

I've now updated my project https://github.com/erikw/taiga-stats to use this release :)

yakky · 2018-02-03T12:01:16Z

Nice tool!
Do you mind adding a comment here #67 to list it?
I would like to start a section of tools and libraries using python-taiga to help people discovery nice stuff

erikw · 2018-02-03T19:33:09Z

Good idea!

yakky force-pushed the feature/pagination branch from c893208 to 7a57741 Compare December 30, 2017 13:07

erikw reviewed Dec 30, 2017

View reviewed changes

yakky force-pushed the feature/pagination branch from 329a2cb to 8f2bb5b Compare December 30, 2017 15:26

Add pagination support for list method

cd70698

yakky force-pushed the feature/pagination branch from 8f2bb5b to cd70698 Compare December 30, 2017 19:59

yakky requested a review from fmarco December 30, 2017 20:19

carloratm suggested changes Jan 5, 2018

View reviewed changes

fmarco reviewed Jan 5, 2018

View reviewed changes

Fixes as per carloratm comments [ci skip]

b805afb

yakky force-pushed the feature/pagination branch from b656ed1 to 5531d32 Compare January 7, 2018 13:17

Add pagination tests

499534f

yakky force-pushed the feature/pagination branch from 5531d32 to 499534f Compare January 7, 2018 13:21

carloratm approved these changes Jan 10, 2018

View reviewed changes

yakky merged commit 618cb2a into master Jan 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preliminary pagination support API #61

Preliminary pagination support API #61

yakky commented Dec 30, 2017

erikw Dec 30, 2017

yakky Dec 30, 2017

erikw Dec 30, 2017

yakky Dec 30, 2017

yakky commented Dec 30, 2017

yakky commented Dec 30, 2017

erikw commented Dec 30, 2017

erikw commented Dec 30, 2017

yakky commented Dec 30, 2017 •

edited

erikw commented Dec 30, 2017

yakky commented Dec 30, 2017 •

edited

yakky commented Dec 30, 2017

erikw commented Dec 30, 2017

yakky commented Dec 30, 2017

carloratm Jan 5, 2018

carloratm Jan 5, 2018

fmarco left a comment

yakky commented Jan 5, 2018

yakky commented Jan 7, 2018

codecov bot commented Jan 7, 2018 •

edited

erikw commented Jan 10, 2018

yakky commented Jan 10, 2018

erikw commented Jan 28, 2018

yakky commented Jan 28, 2018

yakky commented Jan 30, 2018

erikw commented Feb 2, 2018

yakky commented Feb 3, 2018

erikw commented Feb 3, 2018

		@@ -169,6 +169,27 @@ You can also specify filters

		tasks = api.tasks.list(project=1)

		By defaults list returns all objects, eventually getting the


		.. code:: python

		tasks_page_1 = api.tasks.list(lazy_pagination) # Will only return page 1


		tasks = api.tasks.list(paginate=False)

		.. warning:: be aware that if the unpaginated results may exceed

Preliminary pagination support API #61

Preliminary pagination support API #61

Conversation

yakky commented Dec 30, 2017

erikw Dec 30, 2017

Choose a reason for hiding this comment

yakky Dec 30, 2017

Choose a reason for hiding this comment

erikw Dec 30, 2017

Choose a reason for hiding this comment

yakky Dec 30, 2017

Choose a reason for hiding this comment

yakky commented Dec 30, 2017

yakky commented Dec 30, 2017

erikw commented Dec 30, 2017

erikw commented Dec 30, 2017

yakky commented Dec 30, 2017 • edited

erikw commented Dec 30, 2017

yakky commented Dec 30, 2017 • edited

yakky commented Dec 30, 2017

erikw commented Dec 30, 2017

yakky commented Dec 30, 2017

carloratm Jan 5, 2018

Choose a reason for hiding this comment

carloratm Jan 5, 2018

Choose a reason for hiding this comment

fmarco left a comment

Choose a reason for hiding this comment

yakky commented Jan 5, 2018

yakky commented Jan 7, 2018

codecov bot commented Jan 7, 2018 • edited

Codecov Report

erikw commented Jan 10, 2018

yakky commented Jan 10, 2018

erikw commented Jan 28, 2018

yakky commented Jan 28, 2018

yakky commented Jan 30, 2018

erikw commented Feb 2, 2018

yakky commented Feb 3, 2018

erikw commented Feb 3, 2018

yakky commented Dec 30, 2017 •

edited

yakky commented Dec 30, 2017 •

edited

codecov bot commented Jan 7, 2018 •

edited