[Feature] Add update_repo_settings function to HfApi #2447

[WizKnight]

This PR adds a new function update_repo_settings to the HfApi class, allowing users to programmatically update various repository settings, including gating.

[WizKnight]

Hey @Wauplin, can you please go through this added changes and let me know if I'm in the right direction??
Also the tests are failing locally with this changes.

[hanouticelina]

Hello @WizKnight, thank you for this PR :) I left few comments, also I noticed that HfApi.upload_large_folder has been removed. This function is still present in the main branch and appears to be in use. Did you remove it for some reason?

[hanouticelina]

The following commented line can be removed

[HuggingFaceDocBuilderDev]

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.

[Wauplin]

Hey there, thanks for your contribution @WizKnight! I've pushed some commits to fix your merging issues. I don't know what happened but it looks like you did not start a new branch from the main branch (but maybe from your previous one?) and then a merge conflict did not go well. Anyway, should be good now if you pull the latest changes.

Also posted a few more comments in addition to @hanouticelina 's review. Let us know if you have any question :)

[Wauplin]

Hi @WizKnight, thanks for the iteration! I've re-reviewed your work and left a few more comments. Once those are addressed, we should be good to merge :)

[Wauplin]

Thanks for the changes @WizKnight! Everything looks good to me now logic-wise :)

It looks like you still have some minor styling issues. Can you pull the latest version (I've merged from main), run make style locally to format the code, run make quality to check everything's correct and then commit the changes. This should solve the current issues. Thanks!

[hanouticelina]

Hi @WizKnight, thanks so much for your interations on this PR! 🤗 I've left some comments regarding the failing tests

[hanouticelina]

The test was failing with a Bad Request error because repo_type was not specified here. This works fine for model repos, as they are the default repo type. For dataset repos, we need to explicitly provide the repo_type.

Suggested change

	def test_update_dataset_repo_settings(self, repo_url: RepoUrl):
	repo_id = repo_url.repo_id
	for gated_value in ["auto", "manual", False]:
	self._api.update_repo_settings(repo_id=repo_id, gated=gated_value)
	info = self._api.dataset_info(repo_id, expand="gated")
	assert info.gated == gated_value
	def test_update_dataset_repo_settings(self, repo_url: RepoUrl):
	repo_id = repo_url.repo_id
	repo_type = repo_url.repo_type
	for gated_value in ["auto", "manual", False]:
	self._api.update_repo_settings(repo_id=repo_id, repo_type=repo_type, gated=gated_value)
	info = self._api.dataset_info(repo_id, expand="gated")
	assert info.gated == gated_value

[hanouticelina]

Same comment as above, we need to explicitly provide repo_type. Also, you probably got a BadRequestError here, because the concept of "gated" repositories does not apply to spaces. Only models and datasets can be gated. For spaces, the only valid value for the gated parameter is False

[Wauplin]

I think we can drop the test for Space repo_type then, thanks for looking into it @hanouticelina :) Testing on model and dataset is already more than enough 👍

[WizKnight]

Okay then @Wauplin :) , I'll remove the space_repo_type test and commit again.

[WizKnight]

Hey @hanouticelina, thanks for taking a look at the space_repo_settings test. I removed it in the latest commit, as @Wauplin recommended. Also, I added repo_type to dataset_repo_settings based on your suggestion.

[Wauplin]

Thanks @WizKnight! Almost looking good now :) I just realized there is a docstring missing. Could you add one to the method so that it's well documented for users? You can take inspiration from this one. No need to write one as complete as this one but at least have:

1. a short explanation
2. an args section
3. a raises section

I promise, this is the last step to have a clean and self-contained PR!

[WizKnight]

Thanks @WizKnight! Almost looking good now :) I just realized there is a docstring missing. Could you add one to the method so that it's well documented for users? You can take inspiration from this one. No need to write one as complete as this one but at least have:

1. a short explanation
2. an args section
3. a raises section

I promise, this is the last step to have a clean and self-contained PR!

Docstring is added, it's good to go now.

[Wauplin]

minor comments

[Wauplin]

Everything looks good to me! Thanks for your patience @WizKnight! 😄
Let's wait for a last review from @hanouticelina and we should be good :)

[hanouticelina]

Looks good to me too! thanks again @WizKnight for this! 🎉 The failing test is not related to your PR, let me check that

[WizKnight]

Thank you so much, @Wauplin & @hanouticelina !! 🤗 I truly appreciate your guidance and patience throughout this process.

@Wauplin, I'm constantly learning from you, and I'm excited to tackle new challenges. I'd be grateful if you could assign me another task once this PR is successfully closed.

[hanouticelina]

Failing tests are unrelated so let's get this merged :)

[Wauplin]

Hi @WizKnight, I just created #2537 which would be the follow-up work to do after this PR. Interested in working on it?

[WizKnight]

Hi @WizKnight, I just created #2537 which would be the follow-up work to do after this PR. Interested in working on it?

Hi @Wauplin, I would love to. This looks interesting!!