[Bug] [API] make pagination optional for GET /api/user/repos #11800
Comments
lafriks
added
the
issue/critical
|
Disagree, the mentioned comment clearly points at issue in Drone implementation which is missing pagination enumeration. We don't explicitly require pagination options to be set and with or without we need some sane limit on what we return. The instance might have 10 repos or it might have 2000 repos. |
|
I'd like to have both |
|
-> I'll take the gitea part |
|
We definitely should NOT return all repos on API, instances can be small or can be huge. This is not a bug but breaking change on our part since we added pagination; tools should now be updated to use it properly. |
|
@CirnoT yeah but we can not just break drone integration. We can add deprecation notice and ask for drone developers to use paging and later in next version drop that. I think drone integration is used quite extensively to just break it |
|
-> #11808 and for the DOS at witch point it get critical, we could add a hardcoded lim at 50/100/200 ... |
|
sadly I dont know how to "depricate" a optional value to be required ... think the best way is just inform huge project that they have to implement paggination for v1.13 now and switch in v1.13 ? |
|
#11805 ... |
|
@techknowlogick Yes but currently we do return all repos in versions <=v1.11 |
|
I think 30 is a sane default till a fix is pushed to |
|
At my company git I have about 100 repos so that will break drone integration for me ;) |
|
And as not everyone read changelog it will make a lot users not knowing why repos is not syncing anymore in drone and most probably they will complain to drone devs |
|
To give a point it is already creating confusion, see article comment: https://dev.to/ruanbekker/self-hosted-cicd-with-gitea-and-drone-ci-200l |
|
I know that and I will do that but for many this will cause confusion, someone should really send PR to fix drone/gitea integration before we release 1.12 |
|
Are the drone developers even aware of this ongoing discussion? I too feel like deprecation first and removal later is the right approach. Maybe allow turning off compat for deprecated features in config? |
|
@ptman there is already PR that will fix drone support so that no changes in drone are needed |
* Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos`
* Rework ListMyRepos to use models.SearchRepository
ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:
* Limit for pagination did not work because accessible repos would always be appended
* The amount of pages was incorrect if one were to calculate it
* When paginating, all accessible repos would be shown on every page
Hopefully it should now work properly. Fixes #11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.
Co-authored-by: Lauris BH <lauris@nix.lv>
* Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos`
* Rework ListMyRepos to use models.SearchRepository
ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:
* Limit for pagination did not work because accessible repos would always be appended
* The amount of pages was incorrect if one were to calculate it
* When paginating, all accessible repos would be shown on every page
Hopefully it should now work properly. Fixes go-gitea#11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.
Co-authored-by: Lauris BH <lauris@nix.lv>
(cherry picked from commit 0159851)
* Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos`
* Rework ListMyRepos to use models.SearchRepository
ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:
* Limit for pagination did not work because accessible repos would always be appended
* The amount of pages was incorrect if one were to calculate it
* When paginating, all accessible repos would be shown on every page
Hopefully it should now work properly. Fixes #11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.
Co-authored-by: Lauris BH <lauris@nix.lv>
(cherry picked from commit 0159851)
* Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos`
* Rework ListMyRepos to use models.SearchRepository
ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that:
* Limit for pagination did not work because accessible repos would always be appended
* The amount of pages was incorrect if one were to calculate it
* When paginating, all accessible repos would be shown on every page
Hopefully it should now work properly. Fixes go-gitea#11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set.
Co-authored-by: Lauris BH <lauris@nix.lv>
come up with #9452 in v1.12
ref #9452 (comment)
The text was updated successfully, but these errors were encountered: