packngo icon indicating copy to clipboard operation
packngo copied to clipboard

Pagination

Open t0mk opened this issue 6 years ago • 8 comments

partially done in #64, #87

Paginated resources must have the List method modified to sth like List(..., lo *ListOptions). In the ListOptions struct, user can specify what page, and per_page she wants to fetch.

ListOptions also have Include and Exclude and if PerPage is 0 (default, all resource will be fetch, with specified Includes or Excludes). If the ListOptions pointer is nil, all resources will be fetched with default Includes and Excludes.

currently paginated:

  • [x] Devices
  • [x] Volumes
  • [ ] Events
  • [ ] Hardware Reservations
  • [x] Organizations
  • [x] Projects
  • [ ] Transfers
  • [ ] Users

According to the api json, these resoruces have "page" param which should indicate that those are paginated. Not sure about this though.

$.paths./projects/{id}/storage.get.parameters.2.name : page $.paths./organizations/{id}/transfers.get.parameters.2.name : page $.paths./users.get.parameters.1.name : page $.paths./projects.get.parameters.1.name : page $.paths./organizations/{id}/invitations.get.parameters.1.name : page $.paths./organizations/{id}/projects.get.parameters.2.name : page $.paths./volumes/{id}/events.get.parameters.2.name : page $.paths./projects/{id}/plans.get.parameters.2.name : page $.paths./projects/{id}/memberships.get.parameters.1.name : page $.paths./organizations/{id}/devices.get.parameters.2.name : page $.paths./projects/{id}/devices.get.parameters.2.name : page $.paths./projects/{id}/hardware-reservations.get.parameters.2.name : page $.paths./projects/{id}/facilities.get.parameters.2.name : page $.paths./events.get.parameters.1.name : page $.paths./projects/{id}/invitations.get.parameters.1.name : page $.paths./organizations.get.parameters.3.name : page $.paths./devices/{id}/events.get.parameters.2.name : page $.paths./invitations.get.parameters.1.name : page $.paths./projects/{id}/events.get.parameters.2.name : page

t0mk avatar Mar 06 '18 13:03 t0mk

oh actually I had some work to get this done, but its ugly and not a way I want to go in. I filed an api issue to support pagination info via Link headers ala github api. I will ping @nathangoulding on it since he was out while I filed. There done :D.

mmlb avatar Mar 06 '18 15:03 mmlb

ListOptions is fine, but I agree with @alexellis and think we should return all the resources by default. I'd like to hear your thoughts too.

mmlb avatar Mar 06 '18 15:03 mmlb

@mmlb I'm fine with returning all by default.

In the API, there is the "meta" map in the list responses, maybe we could use that instead of Link header. It looks like:

{'self': {'href': '/projects?page=1'}, 'first': {'href': '/projects?page=1'}, 'total': 4, 'current_page': 1, 'last_page': 1, 'last': {'href': '/projects?page=1'}, 'previous': None, 'next': None}

We could do sth like

type Meta struct {
	Self           *Href `json:"self"`
	First          *Href `json:"first"`
	Last           *Href `json:"last"`
	Previous       *Href `json:"previous,omitempty"`
	Next           *Href `json:"next,omitempty"`
	Total          int   `json:"total"`
	CurrentPageNum int   `json:"current_page"`
	LastPageNum    int   `json:"last_page"`
}

type devicesRoot {
	Devices []Device `json:"devices"`
        Meta      Meta   `json:"meta"`
 }

And then make the listing methods work with the Meta.

t0mk avatar Mar 07 '18 12:03 t0mk

Yeah, but then all the structs need to be changed to add that, and honestly I'd rather do it transparently. But I'm not sure how quickly we can get the Link headers going, there's higher priority stuff in progress. My one bit of feedback so far is to make the Meta struct private.

mmlb avatar Mar 12 '18 01:03 mmlb

partially done in #64

Paginated resources:

  • [x] Devices
  • [x] Volumes
  • [ ] Events
  • [ ] Hardware Reservations
  • [ ] Organizations
  • [x] Projects
  • [ ] Transfers
  • [ ] Users

t0mk avatar Mar 30 '18 07:03 t0mk

should have Devices,Volumes,Events, Hardware Reservations,Organizations,Projects,Users

without Memberships,Notifications,Plans,Transfers,Facilities

reporting a request

mberrueta avatar Jul 17 '18 14:07 mberrueta

@t0mk I took a look at https://api.packet.net/api-docs and searched for per_page which reveals the following "paginated" end-points:

  • [x] Devices
    • [x] /projects/{id}/devices
  • [x] Volumes
    • [x] /projects/{id}/storage
  • [x] Events
    • [x] /projects/x/events
    • [x] /devices/x/events
  • [x] Hardware Reservations
    • [x] /projects/{id}/hardware-reservations
  • [x] Organizations
    • [x] /organizations
  • [x] Projects
    • [x] /projects
  • [x] Users
    • [x] /users

We appear to have some pagination in Packngo that does not exist in the API:

  • [x] /projects/{id}/bgp/sessions

Does this break the List function in this case?

We also have accurate pagination in packngo for endpoints that do not appear in the API spec:

  • Organizations
    • [x] /organizations/x/events

This search also revealed some API endpoints that have not been implemented in Packngo (yet):

  • Devices - NOT IMPLEMENTED

    • [ ] /organizations/{id}/devices
  • Events - NOT IMPLEMENTED

    • [ ] /volumes/x/events
    • [ ] /events (alias for /user/events)
  • Invitiations - NOT IMPLEMENTED

    • [ ] /projects/{id}/invitations
    • [ ] /organizations/{id}/invitations
    • [ ] /invitations
  • Projects - NOT IMPLEMENTED

    • [ ] /organizations/{id}/projects
  • Memberships - NOT IMPLEMENTED

    • [ ] /projects/{id}/memberships
  • Project Plans - NOT IMPLEMENTED

    • [ ] /projects/{id}/plans

In inspecting these end-points more closely, it appears that the API spec is reporting pagination in cases where there is none:

  • Facilities

    • [ ] /project/x/facilities
    • [ ] /organizations/x/facilities
  • Transfers

    • [ ] /organizations/{id}/transfers

I found a more accurate way to approach this is to search for "$ref": "#/definitions/Meta" in the swagger spec, and to correlate those *List types back to any endpoint that returns that List type.

I'll ping the API team regarding the spec discrepancies.

displague avatar Jul 22 '20 19:07 displague

We appear to have some pagination in Packngo that does not exist in the API: /projects/{id}/bgp/sessions Does this break the List function in this case?

The bgp session test succeeds (it cointains the list), but I've not tested the pagination there.

PACKNGO_DEBUG=1 PACKNGO_TEST_ACTUAL_API=1 go test -v --run=TestAccBGPSession

We also have accurate pagination in packngo for endpoints that do not appear in the API spec: Organizations /organizations/x/events

That's not good. Are you sure then that the API spec is accurate for the deployed API version?

In inspecting these end-points more closely, it appears that the API spec is reporting pagination in cases where there is none:

dtto

This search also revealed some API endpoints that have not been implemented in Packngo (yet):

If there's a need to do any of the not-implemented API endpoints, create a separate issue please.

t0mk avatar Jul 23 '20 07:07 t0mk