Re: [Pulp-dev] Pagination Requirements and Defaults?
Thanks everyone for the feedback. It generated this story which would switch the default to LimitOffsetPagination. Please raise any concerns you have with Pulp making this switch soon. https://pulp.plan.io/issues/5324 On Wed, Aug 21, 2019 at 10:27 AM Justin Sherrill wrote: > > On 8/20/19 12:17 PM, Brian Bouterse wrote: > > Recently with pulp_ansible, users were interested in using pagination with > LimitOffsetPagination [0]. Pulp currently defaults to PageNumberPagination. > I looked at our current DRF defaults, and I noticed two things. > > 1. We default to the not-as-common PageNumberPagination based on examples > in the drf docs. > 2. We customize it here [1] in various ways. > > Can someone help me remember why these pagination style choices were made > or where the requirements came from? > Would our bindings work with a LimitOffsetPagination style? > What use cases drove the use and customization in this area? > > Also, @katello how would a pagination style change (like switching to > LimitOffsetPagination) affect you? > > Speaking for katello, we'd have to change a few lines of code, but it > seems like it would be minimal. > > > Thanks for any info you can provide. Maybe what we have right now is just > what we need, but I'm not sure. > > -Brian > > [0]: > https://www.django-rest-framework.org/api-guide/pagination/#setting-the-pagination-style > [1]: > https://github.com/pulp/pulpcore/blob/master/pulpcore/app/pagination.py > > ___ > Pulp-dev mailing > listPulp-dev@redhat.comhttps://www.redhat.com/mailman/listinfo/pulp-dev > > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] Pagination Requirements and Defaults?
On 8/20/19 12:17 PM, Brian Bouterse wrote: Recently with pulp_ansible, users were interested in using pagination with LimitOffsetPagination [0]. Pulp currently defaults to PageNumberPagination. I looked at our current DRF defaults, and I noticed two things. 1. We default to the not-as-common PageNumberPagination based on examples in the drf docs. 2. We customize it here [1] in various ways. Can someone help me remember why these pagination style choices were made or where the requirements came from? Would our bindings work with a LimitOffsetPagination style? What use cases drove the use and customization in this area? Also, @katello how would a pagination style change (like switching to LimitOffsetPagination) affect you? Speaking for katello, we'd have to change a few lines of code, but it seems like it would be minimal. Thanks for any info you can provide. Maybe what we have right now is just what we need, but I'm not sure. -Brian [0]: https://www.django-rest-framework.org/api-guide/pagination/#setting-the-pagination-style [1]: https://github.com/pulp/pulpcore/blob/master/pulpcore/app/pagination.py ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] Pagination Requirements and Defaults?
Brian Bouterse writes: > Recently with pulp_ansible, users were interested in using pagination with > LimitOffsetPagination [0]. Pulp currently defaults to PageNumberPagination. > I looked at our current DRF defaults, and I noticed two things. > > 1. We default to the not-as-common PageNumberPagination based on examples > in the drf docs. > 2. We customize it here [1] in various ways. > > Can someone help me remember why these pagination style choices were made > or where the requirements came from? > Would our bindings work with a LimitOffsetPagination style? > What use cases drove the use and customization in this area? > > Also, @katello how would a pagination style change (like switching to > LimitOffsetPagination) affect you? > > Thanks for any info you can provide. Maybe what we have right now is just > what we need, but I'm not sure. > With respect to page number/size vs limit/offset, I'm neutral and don't see any reason our clients couldn't work equally well with either. If you're looking into changes in this area, I only suggest that you would keep it so that there is always a page size/limit applied by default, whether or not the client requested it. Ideally, a small-ish limit to discourage that developers could forget about pagination until it's too late. This is to avoid a problem we've seen several times with Pulp 2.x now: - search API by default doesn't paginate at all - client writes naive search code which ignores pagination, requests all available data at once - client tests their code against a small Pulp test server or repo and it appears to work fine - later, when code is used against a real Pulp server/repo (=> much larger amount of data) or as the amount of data in Pulp grows, search becomes slow and prone to OOM crashes - client blames Pulp for being slow, or service owners for not putting enough RAM on the Pulp servers > > [0]: > https://www.django-rest-framework.org/api-guide/pagination/#setting-the-pagination-style > [1]: https://github.com/pulp/pulpcore/blob/master/pulpcore/app/pagination.py ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] Pagination Requirements and Defaults?
On Tue, Aug 20, 2019 at 1:06 PM David Davis wrote: > #3801 only describes the move from CursorPagination to > PageNumberPagination. I don't think we considered > using LimitOffsetPagination. > > That's right David. The CursorPagination was picked randomly when we were implementing the initial base classes for Pulp 3[0]. It was part of this commit[1]. I am interested in users such as Katello providing feedback on whether or not they could switch to using the LimitOffsetPagination[2]. [0] https://pulp.plan.io/issues/1873 [1] https://github.com/pulp/pulpcore/commit/d03d73c9714aa4c21c296b3749c929f688ea2f89 [2] https://www.django-rest-framework.org/api-guide/pagination/#limitoffsetpagination > David > > > On Tue, Aug 20, 2019 at 1:01 PM Dennis Kliban wrote: > >> On Tue, Aug 20, 2019 at 12:17 PM Brian Bouterse >> wrote: >> >>> Recently with pulp_ansible, users were interested in using pagination >>> with LimitOffsetPagination [0]. Pulp currently defaults to >>> PageNumberPagination. I looked at our current DRF defaults, and I noticed >>> two things. >>> >>> 1. We default to the not-as-common PageNumberPagination based on >>> examples in the drf docs. >>> 2. We customize it here [1] in various ways. >>> >>> Can someone help me remember why these pagination style choices were >>> made or where the requirements came from? >>> >> >> I believe the motivation is described here: >> https://pulp.plan.io/issues/3801 >> >> >>> Would our bindings work with a LimitOffsetPagination style? >>> >> >> Yes, the bindings will work with anything that uses query parameters for >> pagination. >> >> >>> What use cases drove the use and customization in this area? >>> >>> Also, @katello how would a pagination style change (like switching to >>> LimitOffsetPagination) affect you? >>> >>> Thanks for any info you can provide. Maybe what we have right now is >>> just what we need, but I'm not sure. >>> >>> -Brian >>> >>> [0]: >>> https://www.django-rest-framework.org/api-guide/pagination/#setting-the-pagination-style >>> [1]: >>> https://github.com/pulp/pulpcore/blob/master/pulpcore/app/pagination.py >>> ___ >>> Pulp-dev mailing list >>> Pulp-dev@redhat.com >>> https://www.redhat.com/mailman/listinfo/pulp-dev >>> >> ___ >> Pulp-dev mailing list >> Pulp-dev@redhat.com >> https://www.redhat.com/mailman/listinfo/pulp-dev >> > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] Pagination Requirements and Defaults?
#3801 only describes the move from CursorPagination to PageNumberPagination. I don't think we considered using LimitOffsetPagination. David On Tue, Aug 20, 2019 at 1:01 PM Dennis Kliban wrote: > On Tue, Aug 20, 2019 at 12:17 PM Brian Bouterse > wrote: > >> Recently with pulp_ansible, users were interested in using pagination >> with LimitOffsetPagination [0]. Pulp currently defaults to >> PageNumberPagination. I looked at our current DRF defaults, and I noticed >> two things. >> >> 1. We default to the not-as-common PageNumberPagination based on examples >> in the drf docs. >> 2. We customize it here [1] in various ways. >> >> Can someone help me remember why these pagination style choices were made >> or where the requirements came from? >> > > I believe the motivation is described here: > https://pulp.plan.io/issues/3801 > > >> Would our bindings work with a LimitOffsetPagination style? >> > > Yes, the bindings will work with anything that uses query parameters for > pagination. > > >> What use cases drove the use and customization in this area? >> >> Also, @katello how would a pagination style change (like switching to >> LimitOffsetPagination) affect you? >> >> Thanks for any info you can provide. Maybe what we have right now is just >> what we need, but I'm not sure. >> >> -Brian >> >> [0]: >> https://www.django-rest-framework.org/api-guide/pagination/#setting-the-pagination-style >> [1]: >> https://github.com/pulp/pulpcore/blob/master/pulpcore/app/pagination.py >> ___ >> Pulp-dev mailing list >> Pulp-dev@redhat.com >> https://www.redhat.com/mailman/listinfo/pulp-dev >> > ___ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
Re: [Pulp-dev] Pagination Requirements and Defaults?
On Tue, Aug 20, 2019 at 12:17 PM Brian Bouterse wrote: > Recently with pulp_ansible, users were interested in using pagination with > LimitOffsetPagination [0]. Pulp currently defaults to PageNumberPagination. > I looked at our current DRF defaults, and I noticed two things. > > 1. We default to the not-as-common PageNumberPagination based on examples > in the drf docs. > 2. We customize it here [1] in various ways. > > Can someone help me remember why these pagination style choices were made > or where the requirements came from? > I believe the motivation is described here: https://pulp.plan.io/issues/3801 > Would our bindings work with a LimitOffsetPagination style? > Yes, the bindings will work with anything that uses query parameters for pagination. > What use cases drove the use and customization in this area? > > Also, @katello how would a pagination style change (like switching to > LimitOffsetPagination) affect you? > > Thanks for any info you can provide. Maybe what we have right now is just > what we need, but I'm not sure. > > -Brian > > [0]: > https://www.django-rest-framework.org/api-guide/pagination/#setting-the-pagination-style > [1]: > https://github.com/pulp/pulpcore/blob/master/pulpcore/app/pagination.py > ___ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
[Pulp-dev] Pagination Requirements and Defaults?
Recently with pulp_ansible, users were interested in using pagination with LimitOffsetPagination [0]. Pulp currently defaults to PageNumberPagination. I looked at our current DRF defaults, and I noticed two things. 1. We default to the not-as-common PageNumberPagination based on examples in the drf docs. 2. We customize it here [1] in various ways. Can someone help me remember why these pagination style choices were made or where the requirements came from? Would our bindings work with a LimitOffsetPagination style? What use cases drove the use and customization in this area? Also, @katello how would a pagination style change (like switching to LimitOffsetPagination) affect you? Thanks for any info you can provide. Maybe what we have right now is just what we need, but I'm not sure. -Brian [0]: https://www.django-rest-framework.org/api-guide/pagination/#setting-the-pagination-style [1]: https://github.com/pulp/pulpcore/blob/master/pulpcore/app/pagination.py ___ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
