-
Notifications
You must be signed in to change notification settings - Fork 539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Pagination in Tunnels API #1206
Conversation
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
changelog detected ✅ |
Codecov Report
@@ Coverage Diff @@
## master #1206 +/- ##
==========================================
+ Coverage 49.36% 49.39% +0.02%
==========================================
Files 128 128
Lines 12430 12437 +7
==========================================
+ Hits 6136 6143 +7
Misses 4890 4890
Partials 1404 1404
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
thanks for the PR @EngHabu. this is conceptually where we want to be but the implementation is a little off with the other parts of the library. we have the next gen client docs that outline the way we want to handle these but in short:
we have quite a quite methods now that implement this so check out Line 154 in 1385fe5
if you can swap to these conventions, i'm 👍 on getting this one merged. |
@jacobbednarz, thank you for the response. Are you ok breaking the current signature by returning a 3rd value (ResultInfo)? or should I leave this part out? |
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
yep, that's totally fine here since we're moving towards the expected conventions 👍 |
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
… into pagination-tunnel
Can you take a look now, @jacobbednarz? |
thanks @EngHabu, appreciate it! |
This functionality has been released in v0.62.0. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
Signed-off-by: Haytham Abuelfutuh haytham@afutuh.com
Description
GET Tunnels API supports pagination but the cloudflare-go client doesn't. I copied the way it's done from Zone Contexts into this API. Generally speaking I think all APIs that support pagination should ideally support
ReqOption
sExample:
Has your change been tested?
Using the fork, we have deployed a service that syncs the status of the channels to a local DB and now is able to iterate through all tunnels we've (100+ in total)
Types of changes
What sort of change does your code introduce/modify?
Checklist: