PLA-3994: list pagination

[talltea]

Citrine Python PR

Description

This is still WIP
This PR attempts to address these two main issues:

1. the list endpoints don't make use of the module type filtering functionality of the back end and rely on serialization errors to filter modules client side
2. the pagination of the list endpoint stops prematurely when the number modules on the client side is less than the number requested by page after the client does filtering and deserialization

Solution summaries:

1. add a _module_type field that can be used in the list endpoint (so far only to predictors, but I'm also going to add it to other supported modules)
2. refactor the pagination code to allow for use of the next field provided in the backend response field to determine if we are at the end of the pagination

Things I'm still working on:

1. Editing docstrings and type annotations
2. Ensuring full test coverage

PR Type:

• Breaking change (fix or feature that would cause existing functionality to change)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Adherence to team decisions

• I have added tests for 100% coverage
• I have written Numpy-style docstrings for every method and class.
• I have communicated the downstream consequences of the PR to others.
• I have bumped the version in setup.py

[bfolie]

Assuming the paginator works as expected in the backend (if page is too large then it returns an empty list and no value of next_uri), then this approach looks like it will work.

[talltea]

Assuming the paginator works as expected in the backend (if page is too large then it returns an empty list and no value of next_uri), then this approach looks like it will work.

I've confirmed this branch works with project.predictors.list() against development as expected

[jtharris]

This is great and fixes some pretty critical issues, but I'd also like to think about few other things:

1. Are we actually using the next url to fetch the next page? There are lots of benefits to doing this, a big one being that we don't have to thread any knowledge of paging throughout the client.

2. Are there any other missing module types? The specific bug that we are addressing in development is with design spaces, not predictors, right?

[andyczerwonka]

1. Are we actually using the `next` url to fetch the next page?  There are lots of benefits to doing this, a big one being that we don't have to thread any knowledge of paging throughout the client.

I would like to capture this, as I think this is the whole point of the API and something we should be doing. Saying that, I'd like to get over the dev0 and log a ticket for this.

[talltea]

1. Are we actually using the next url to fetch the next page? There are lots of benefits to doing this, a big one being that we don't have to thread any knowledge of paging throughout the client.

We're not currently using it. I agree there's a lot of benefit of using it, but I didn't want to pull that change in to this PR because that would have been a larger refactor.

2. Are there any other missing module types? The specific bug that we are addressing in development is with design spaces, not predictors, right?

Absolutely. I missed that in my PR description as a todo, but I will be going through and adding the other module types supported. The ones I see are DESIGN_SPACE, PROCESSOR, PREDICTOR, SCORER, DESIGN_WORKFLOW, CONSTRAINT, OBJECTIVE, SCORE, PERFORMANCE_WORKFLOW

[bfolie]

I think we should add the _module_type field to Workflow. Otherwise LGTM

[talltea]

I think we should add the _module_type field to Workflow. Otherwise LGTM

I ran into a problem with this since the backend distinguishes between performance and design workflows on the list endpoint, but the client doesn't. So it would require a slightly different fix client side. Since that change isn't necessary to get us past the dev0, I was going to handle it in a separate pr.

[andyczerwonka]

Since that change isn't necessary to get us past the dev0, I was going to handle it in a separate pr.

Let's write up a separate issue for that, but yes, I agree. @bfolie are we okay with that?

[jtharris]

Thanks for this!