search

apache / openwhisk-apigateway

Apache OpenWhisk API Gateway service for exposing actions as REST interfaces.
https://openwhisk.apache.org/
Apache License 2.0
64 stars 45 forks source link

Add paging to getTenantAPIs #335

Closed larandersson closed 5 years ago

larandersson commented 5 years ago

This fix adds paging to the v2 getTenantAPIs api by honoring the skip and limit paramaters, if present in the request. This is to address issue #1692 in incubator-openwhisk.

mhamann commented 5 years ago

Hi @larandersson! Thanks for the contribution. The CI is failing currently due to a downstream issue, so that's blocking your PR at the moment.

While you wait for that to be resolved, have you signed the Apache ICLA?

rabbah commented 5 years ago

Yes he's in the roster here http://people.apache.org/unlistedclas.html

rabbah commented 5 years ago

@mhamann what do you suggest to unlock the build?

mhamann commented 5 years ago

@larandersson if you rebase your PR on master, the build should complete successfully.

I think there should also be some tests written for the pagination feature to ensure it works correctly and doesn't regress by future contributions. Something in here would probably make sense: https://github.com/apache/incubator-openwhisk-apigateway/tree/master/tests/scripts/lua/management/lib

rabbah commented 5 years ago

I rebased this.

rabbah commented 5 years ago

Is this OK to merge now?

mhamann commented 5 years ago

@rabbah there don't appear to be any tests for this new functionality, so my vote is to hold off.

rabbah commented 5 years ago

I don’t see any tests for code under tenants management to begin with, can you point me to them I might not be looking in the right place.

larandersson commented 5 years ago

@mhamann @rabbah I don't see them either. And, just to clarify, this is a bug fix, not new functionality. I suggest opening a separate issue to address the tests discrepancy.

rabbah commented 5 years ago

We can also test the functionality downstream?

larandersson commented 5 years ago

Yes. The main repo would be a good place to test.