Paginate repo likers endpoint #2530
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| for liker in paginate(path, params={}, headers=self._build_hf_headers(token=token)): | ||
| yield User(username=liker["user"], fullname=liker["fullname"], avatar_url=liker["avatarUrl"]) |
There was a problem hiding this comment.
Note that this is unfortunately a breaking change since users cannot do list_repo_likers(...)[0] anymore. It might break some script in the wild. We already had a similar issue when we switched list_models (and datasets/spaces) to paginated results in the past. It was much more painful as it was used everywhere.
Here I'm not sure there is much adoption yet so I would be tempted to merge "as is" and break things rather than having a half-solution for months. @LysandreJik what's your take on this?
For context, I've checked https://github.com/search?q=list_repo_likers&type=code&p=1 and didn't find much third-party integrations.
There was a problem hiding this comment.
I would tend to say that a deprecation cycle rarely hurts but if you're certain it's not being used much yet, then maybe breaking this one is fine
There was a problem hiding this comment.
The problem is that a deprecation cycle on an output type is a bit more annoying. I tried something for list_models in the past (#1143) but we got quite some problems and misunderstanding from users... 😕
There was a problem hiding this comment.
Since there is not so much adoption (compared to list_models() or list_datasets()), imo it's worth merging as is rather than handling a deprecation cycle
tests/test_hf_api.py
Outdated
| # a repo with > 5000 likes | ||
| all_likers = list( | ||
| HfApi().list_repo_likers( | ||
| repo_id="open-llm-leaderboard/open_llm_leaderboard", repo_type="space", token=TOKEN |
There was a problem hiding this comment.
I think passing TOKEN (i.e. a staging token) to production will fail. It's fine not to pass token at all in this case.
There was a problem hiding this comment.
my bad! I forgot to remove it, thanks!
| for liker in paginate(path, params={}, headers=self._build_hf_headers(token=token)): | ||
| yield User(username=liker["user"], fullname=liker["fullname"], avatar_url=liker["avatarUrl"]) |
There was a problem hiding this comment.
I would tend to say that a deprecation cycle rarely hurts but if you're certain it's not being used much yet, then maybe breaking this one is fine
|
Thanks @Wauplin and @LysandreJik for the reviews! based on the discussion here, let's implement the breaking change directly, I'm merging the PR 😄 |
Related to #2508 and #2506.
The related test was not modified. To have a robust test, we should maybe use an existing repo with enough likers. WDYT? Do you have any suggestion of such repo we can rely on for this test? @Wauplin