Skip to content
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.

Sign up for GitHub

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

Feature: switch visibility with update_repo_settings #2537 #2541

Merged
merged 11 commits into from
Sep 26, 2024
Merged

Feature: switch visibility with update_repo_settings #2537 #2541

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
de6794c
Enhance `update_repo_settings` to manage repo visibility
WizKnight Sep 14, 2024
eae6028
Merge branch 'main' into feature/switch-visibility-with-repo-settings
WizKnight Sep 16, 2024
5a376f3
Merge branch 'main' into feature/switch-visibility-with-repo-settings
WizKnight Sep 16, 2024
5e3aef1
Merge branch 'main' into feature/switch-visibility-with-repo-settings
WizKnight Sep 19, 2024
e169e96
Enhance `update_repo_settings` to manage repo visibility
WizKnight Sep 19, 2024
edf03d2
Merge branch 'main' into feature/switch-visibility-with-repo-settings
WizKnight Sep 20, 2024
06e31db
Enhance `update_repo_settings` to manage repo visibility
WizKnight Sep 20, 2024
b06e6b4
Enhance `update_repo_settings` to manage repo visibility
WizKnight Sep 20, 2024
e68280c
Enhance `update_repo_settings` to manage repo visibility
WizKnight Sep 26, 2024
93d87a6
Merge branch 'main' into feature/switch-visibility-with-repo-settings
WizKnight Sep 26, 2024
8b13d08
Apply suggestions from code review
Wauplin Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions docs/source/en/guides/repository.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,26 +146,21 @@ an organization). In this section, we will see the settings that you can also co

Some settings are specific to Spaces (hardware, environment variables,...). To configure those, please refer to our [Manage your Spaces](../guides/manage-spaces) guide.

### Update visibility
### Setup gated access and visibility

A repository can be public or private. A private repository is only visible to you or members of the organization in which the repository is located. Change a repository to private as shown in the following:
To enhance control over how repos are utilized, the Hub empowers repo authors to enable **access requests** for their repos, and to set the visibility of the repo to **private**.

```py
>>> from huggingface_hub import update_repo_visibility
>>> update_repo_visibility(repo_id=repo_id, private=True)
```

### Setup gated access
When **access requests** are enabled, users must agree to share their contact information (username and email address) with the repo authors to gain access to the files. A repo with access requests enabled is referred to as a **gated repo**.

To give more control over how repos are used, the Hub allows repo authors to enable **access requests** for their repos. User must agree to share their contact information (username and email address) with the repo authors to access the files when enabled. A repo with access requests enabled is called a **gated repo**.
And a repository can be public or private. A private repository is only visible to you or members of the organization in which the repository is located.

You can set a repo as gated using [`update_repo_settings`]:
You can configure these settings using [`update_repo_settings`]:

```py
>>> from huggingface_hub import HfApi

>>> api = HfApi()
>>> api.update_repo_settings(repo_id=repo_id, gated="auto") # Set automatic gating for a model
>>> api.update_repo_settings(repo_id=repo_id, gated="auto", private=True) # Set automatic gating and visibility for a model
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would structure the documentation as such:

## Change repository settings

(...) current content

### Update visibility

(...) current content + update example

### Setup gated access

(...) current content

Setting a repo as gating and setting repo visibility is 2 different topics, even if related to accessibility. Usually either a repo is public, either private, either public and gated. Setting a repo as private and gated is possible but doesn't make sense.

```

### Rename your repository
Expand Down
25 changes: 18 additions & 7 deletions src/huggingface_hub/hf_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3524,7 +3524,7 @@ def delete_repo(
if not missing_ok:
raise

@validate_hf_hub_args
@_deprecate_method(version="0.29", message="Please use `update_repo_settings` instead.")
hanouticelina marked this conversation as resolved.
Show resolved Hide resolved
def update_repo_visibility(
self,
repo_id: str,
Expand All @@ -3533,7 +3533,7 @@ def update_repo_visibility(
token: Union[str, bool, None] = None,
repo_type: Optional[str] = None,
) -> Dict[str, bool]:
"""Update the visibility setting of a repository.
"""(Deprecated) Update the visibility setting of a repository.
hanouticelina marked this conversation as resolved.
Show resolved Hide resolved

Args:
repo_id (`str`, *optional*):
Expand Down Expand Up @@ -3581,13 +3581,16 @@ def update_repo_settings(
self,
repo_id: str,
*,
gated: Literal["auto", "manual", False] = False,
gated: Optional[Literal["auto", "manual", False]] = None,
private: Optional[bool] = None,
token: Union[str, bool, None] = None,
repo_type: Optional[str] = None,
) -> None:
"""
Update the gated settings of a repository.
To give more control over how repos are used, the Hub allows repo authors to enable **access requests** for their repos.
Update the settings of a repository, including gated access and visibility.

To give more control over how repos are used, the Hub allows repo authors to enable
access requests for their repos, and also to set the visibility of the repo to private.

Args:
repo_id (`str`):
Expand All @@ -3597,6 +3600,8 @@ def update_repo_settings(
* "auto": The repository is gated, and access requests are automatically approved or denied based on predefined criteria.
* "manual": The repository is gated, and access requests require manual approval.
* False (default): The repository is not gated, and anyone can access it.
hanouticelina marked this conversation as resolved.
Show resolved Hide resolved
private (`bool`, *optional*, defaults to `False`):
hanouticelina marked this conversation as resolved.
Show resolved Hide resolved
Whether the model repo should be private.
token (`Union[str, bool, None]`, *optional*):
A valid user access token (string). Defaults to the locally saved token,
which is the recommended method for authentication (see
Expand All @@ -3613,8 +3618,11 @@ def update_repo_settings(
If repo_type is not one of the values in constants.REPO_TYPES.
[`~utils.HfHubHTTPError`]:
If the request to the Hugging Face Hub API fails.
[`~utils.RepositoryNotFoundError`]
If the repository to download from cannot be found. This may be because it doesn't exist,
or because it is set to `private` and you do not have access.
"""
if gated not in ["auto", "manual", False]:
if gated is not None and gated not in ["auto", "manual", False]:
hanouticelina marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError(f"Invalid gated status, must be one of 'auto', 'manual', or False. Got '{gated}'.")

if repo_type not in constants.REPO_TYPES:
Expand All @@ -3625,10 +3633,13 @@ def update_repo_settings(
# Build headers
headers = self._build_hf_headers(token=token)

# Preparing the JSON payload for the PUT request
payload = {"gated": gated, "private": private}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prepare the payload in 2 steps:

payload: Dict = {}

if gated is not None:
    if gated not in ["auto", "manual", False]:
        raise ValueError(f"Invalid gated status, must be one of 'auto', 'manual', or False. Got '{gated}'.")
    payload["gated"] = gated

if private is not None:
    payload["private"] = private

The advantage of doing so is that adding a new parameter will just be a matter of adding a new if statement. Also, it makes it easier to log/print the payload value in case of debugging an issue. Doing a {key: value for key, value in payload.items() if value is not None} is not wrong per-se but makes it slightly less readable and less debuggable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning on a similar approach, but I opted for the dictionary comprehension {key: value for key, value in payload.items() if value is not None} to make the code more efficient.

Copy link
Contributor

@Wauplin Wauplin Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes readibility / maintenance is more important than efficiency :)


r = get_session().put(
url=f"{self.endpoint}/api/{repo_type}s/{repo_id}/settings",
headers=headers,
json={"gated": gated},
json={key: value for key, value in payload.items() if value is not None},
)
hf_raise_for_status(r)

Expand Down
36 changes: 18 additions & 18 deletions tests/test_hf_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ def setUpClass(cls):

def test_repo_id_no_warning():
# tests that passing repo_id as positional arg doesn't raise any warnings
# for {create, delete}_repo and update_repo_visibility
# for {create, delete}_repo and update_repo_settings
api = HfApi(endpoint=ENDPOINT_STAGING, token=TOKEN)

with warnings.catch_warnings(record=True) as record:
repo_id = api.create_repo(repo_name()).repo_id
api.update_repo_visibility(repo_id, private=True)
api.update_repo_settings(repo_id, gated="auto", private=True)
api.delete_repo(repo_id)
assert not len(record)

Expand Down Expand Up @@ -212,26 +212,26 @@ def test_delete_repo_missing_ok(self) -> None:

def test_create_update_and_delete_repo(self):
repo_id = self._api.create_repo(repo_id=repo_name()).repo_id
res = self._api.update_repo_visibility(repo_id=repo_id, private=True)
assert res["private"]
res = self._api.update_repo_visibility(repo_id=repo_id, private=False)
assert not res["private"]
self._api.update_repo_settings(repo_id=repo_id, private=True)
hanouticelina marked this conversation as resolved.
Show resolved Hide resolved
info = self._api.repo_info(repo_id)
if info is not None:
assert info.private
self._api.delete_repo(repo_id=repo_id)

def test_create_update_and_delete_model_repo(self):
repo_id = self._api.create_repo(repo_id=repo_name(), repo_type=constants.REPO_TYPE_MODEL).repo_id
res = self._api.update_repo_visibility(repo_id=repo_id, private=True, repo_type=constants.REPO_TYPE_MODEL)
assert res["private"]
res = self._api.update_repo_visibility(repo_id=repo_id, private=False, repo_type=constants.REPO_TYPE_MODEL)
assert not res["private"]
self._api.update_repo_settings(repo_id=repo_id, private=True, repo_type=constants.REPO_TYPE_MODEL)
info = self._api.model_info(repo_id)
if info is not None:
assert info.private
self._api.delete_repo(repo_id=repo_id, repo_type=constants.REPO_TYPE_MODEL)

def test_create_update_and_delete_dataset_repo(self):
repo_id = self._api.create_repo(repo_id=repo_name(), repo_type=constants.REPO_TYPE_DATASET).repo_id
res = self._api.update_repo_visibility(repo_id=repo_id, private=True, repo_type=constants.REPO_TYPE_DATASET)
assert res["private"]
res = self._api.update_repo_visibility(repo_id=repo_id, private=False, repo_type=constants.REPO_TYPE_DATASET)
assert not res["private"]
self._api.update_repo_settings(repo_id=repo_id, private=True, repo_type=constants.REPO_TYPE_DATASET)
info = self._api.dataset_info(repo_id)
if info is not None:
assert info.private
self._api.delete_repo(repo_id=repo_id, repo_type=constants.REPO_TYPE_DATASET)

def test_create_update_and_delete_space_repo(self):
Expand All @@ -244,10 +244,10 @@ def test_create_update_and_delete_space_repo(self):
repo_id = self._api.create_repo(
repo_id=repo_name(), repo_type=constants.REPO_TYPE_SPACE, space_sdk=sdk
).repo_id
res = self._api.update_repo_visibility(repo_id=repo_id, private=True, repo_type=constants.REPO_TYPE_SPACE)
assert res["private"]
res = self._api.update_repo_visibility(repo_id=repo_id, private=False, repo_type=constants.REPO_TYPE_SPACE)
assert not res["private"]
self._api.update_repo_settings(repo_id=repo_id, private=True, repo_type=constants.REPO_TYPE_SPACE)
info = self._api.space_info(repo_id)
if info is not None:
assert info.private
self._api.delete_repo(repo_id=repo_id, repo_type=constants.REPO_TYPE_SPACE)

def test_move_repo_normal_usage(self):
Expand Down
Loading