Fish Trophy

chipx86 got a fish trophy!

Fish Trophy

Add initial support for paginators for Hosting Service requests.

Review Request #5885 — Created May 27, 2014 and submitted

Information

Review Board
master
880ed5a...

Reviewers

We have functions in HostingService subclasses that need to deal with
paginated data. Right now, they're all handling it in their own ways,
but as we move to wrapping more of these in the web API, it's helpful to
standardize them.

This introduces a set of paginators that can be used and subclassed by
the more specific HostingServices to provide a standardized API for
pagination of data.

APIPaginator is designed to perform API requests and handle pagination
through links returned through the payload or headers in some form.
Subclasses are expected to define how API requests are made and parsed.

ProxyPaginator attaches to another paginator (typically an APIPaginator)
and transforms the results. This allows, for instance, a
HostingServiceClient to return an APIPaginator with raw results, and a
HostingService to wrap it and return structured data derived from those
raw results.

Unit tests passed.

Tested this manually with an accompanying change to GitHub's
get_remote_repositories. I was able to page back and forth and get the
results. I also tested start= and per_page= with that.

Description From Last Updated

What is the client parameter? It doesn't seem to be used.

daviddavid

typo (frmo)

daviddavid

Is this supposed to be public API?

daviddavid
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/hostingsvcs/utils/tests.py
        reviewboard/hostingsvcs/utils/paginator.py
      Ignored Files:
    
    
  2. 
      
reviewbot
  1. This is a review from Review Bot.
      Tool: Pyflakes
      Processed Files:
        reviewboard/hostingsvcs/utils/tests.py
        reviewboard/hostingsvcs/utils/paginator.py
      Ignored Files:
    
    
  2. 
      
david
  1. 
      
  2. reviewboard/hostingsvcs/utils/paginator.py (Diff revision 1)
     
     
     
     
     
    Show all issues

    What is the client parameter? It doesn't seem to be used.

    1. Since subclasses need to make queries on a client (using json_get or api_get or whatever is appropriate), this standardizes accepting it as a parameter, so that subclasses don't have to override the constructor and accept it. The class is useless without it.

    2. Care to document that it's a HostingServiceClient and is used only in subclasses?

  3. Show all issues

    typo (frmo)

  4. 
      
chipx86
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/hostingsvcs/utils/tests.py
        reviewboard/hostingsvcs/utils/paginator.py
      Ignored Files:
    
    
  2. 
      
reviewbot
  1. This is a review from Review Bot.
      Tool: Pyflakes
      Processed Files:
        reviewboard/hostingsvcs/utils/tests.py
        reviewboard/hostingsvcs/utils/paginator.py
      Ignored Files:
    
    
  2. 
      
david
  1. 
      
  2. Show all issues

    Is this supposed to be public API?

    1. Yeah, it's available for subclassing if a paginator needs to be more complex in its processing. (I think we'll have some like that.)

      I can mention it in the docstring.

  3. 
      
chipx86
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/hostingsvcs/utils/tests.py
        reviewboard/hostingsvcs/utils/paginator.py
      Ignored Files:
    
    
  2. 
      
reviewbot
  1. This is a review from Review Bot.
      Tool: Pyflakes
      Processed Files:
        reviewboard/hostingsvcs/utils/tests.py
        reviewboard/hostingsvcs/utils/paginator.py
      Ignored Files:
    
    
  2. 
      
david
  1. Ship It!

  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to master (8ed4055)