Repository: huggingface/huggingface_hub
Pull request #1884 (2023-12-04 11:09:13)

Fix `WebhookPayload` schema + add `WebhooksServer.launch`
This PR:
- fixes an issue with `WebhookPayload`. In Pydantic v2.0 optional fields must be explicitly set to None by default otheriwse it raises an issue. That was happening for the `comment` value that is optional in the payload. Also added a test for it.
- renames `WebhooksServer.run` to `WebhooksServer.launch` to stick to gradio naming. I also allowed users to pass parameters that are forwarded to gradio. I added a deprecation message to `.run` even though it is flagged as an "experimental feature" anyway.

(**EDIT:** failing tests are unrelated)

HuggingFaceDocBuilderDev: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1884). All of your documentation changes will be reflected on that endpoint.

Patch:
diff --git a/src/huggingface_hub/_webhooks_payload.py b/src/huggingface_hub/_webhooks_payload.py
index 784131ea..e4d224cc 100644
--- a/src/huggingface_hub/_webhooks_payload.py
+++ b/src/huggingface_hub/_webhooks_payload.py
@@ -80,7 +80,7 @@ class WebhookPayloadDiscussionChanges(BaseModel):
 class WebhookPayloadComment(ObjectId):
     author: ObjectId
     hidden: bool
-    content: Optional[str]
+    content: Optional[str] = None
     url: WebhookPayloadUrl
 
 
@@ -91,7 +91,7 @@ class WebhookPayloadDiscussion(ObjectId):
     title: str
     isPullRequest: bool
     status: DiscussionStatus_T
-    changes: Optional[WebhookPayloadDiscussionChanges]
+    changes: Optional[WebhookPayloadDiscussionChanges] = None
     pinned: Optional[bool] = None
 
 
@@ -109,7 +109,7 @@ class WebhookPayloadRepo(ObjectId):
 class WebhookPayload(BaseModel):
     event: WebhookPayloadEvent
     repo: WebhookPayloadRepo
-    discussion: Optional[WebhookPayloadDiscussion]
-    comment: Optional[WebhookPayloadComment]
+    discussion: Optional[WebhookPayloadDiscussion] = None
+    comment: Optional[WebhookPayloadComment] = None
     webhook: WebhookPayloadWebhook
     movedTo: Optional[WebhookPayloadMovedTo] = None
diff --git a/src/huggingface_hub/_webhooks_server.py b/src/huggingface_hub/_webhooks_server.py
index 7cc5dd4c..7a8fa061 100644
--- a/src/huggingface_hub/_webhooks_server.py
+++ b/src/huggingface_hub/_webhooks_server.py
@@ -17,9 +17,10 @@ import atexit
 import inspect
 import os
 from functools import wraps
-from typing import TYPE_CHECKING, Callable, Dict, Optional
+from typing import TYPE_CHECKING, Any, Callable, Dict, Optional
 
 from .utils import experimental, is_gradio_available
+from .utils._deprecation import _deprecate_method
 
 
 if TYPE_CHECKING:
@@ -151,14 +152,18 @@ class WebhooksServer:
 
         return _inner_post
 
-    def run(self) -> None:
-        """Starts the Gradio app with the FastAPI server and registers the webhooks."""
+    def launch(self, prevent_thread_lock: bool = False, **launch_kwargs: Any) -> None:
+        """Launch the Gradio app and register webhooks to the underlying FastAPI server.
+
+        Input parameters are forwarded to Gradio when launching the app.
+        """
         ui = self._ui or self._get_default_ui()
 
         # Start Gradio App
         #   - as non-blocking so that webhooks can be added afterwards
         #   - as shared if launch locally (to debug webhooks)
-        self.fastapi_app, _, _ = ui.launch(prevent_thread_lock=True, share=_is_local)
+        launch_kwargs.setdefault("share", _is_local)
+        self.fastapi_app, _, _ = ui.launch(prevent_thread_lock=True, **launch_kwargs)
 
         # Register webhooks to FastAPI app
         for path, func in self.registered_webhooks.items():
@@ -176,7 +181,12 @@ class WebhooksServer:
         message += "\nGo to https://huggingface.co/settings/webhooks to setup your webhooks."
         print(message)
 
-        ui.block_thread()
+        if not prevent_thread_lock:
+            ui.block_thread()
+
+    @_deprecate_method(version="0.23", message="Use `WebhooksServer.launch` instead.")
+    def run(self) -> None:
+        return self.launch()
 
     def _get_default_ui(self) -> "gr.Blocks":
         """Default UI if not provided (lists webhooks and provides basic instructions)."""


Test patch:
diff --git a/tests/test_webhooks_server.py b/tests/test_webhooks_server.py
index fd5364f4..18b6e07d 100644
--- a/tests/test_webhooks_server.py
+++ b/tests/test_webhooks_server.py
@@ -17,7 +17,7 @@ if is_gradio_available():
 
 
 # Taken from https://huggingface.co/docs/hub/webhooks#event
-WEBHOOK_PAYLOAD_EXAMPLE = {
+WEBHOOK_PAYLOAD_CREATE_DISCUSSION = {
     "event": {"action": "create", "scope": "discussion"},
     "repo": {
         "type": "model",
@@ -50,11 +50,49 @@ WEBHOOK_PAYLOAD_EXAMPLE = {
     "webhook": {"id": "6390e855e30d9209411de93b", "version": 3},
 }
 
+WEBHOOK_PAYLOAD_UPDATE_DISCUSSION = {  # valid payload but doesn't have a "comment" value
+    "event": {"action": "update", "scope": "discussion"},
+    "repo": {
+        "type": "space",
+        "name": "Wauplin/leaderboard",
+        "id": "656896965808298301ed7ccf",
+        "private": False,
+        "url": {
+            "web": "https://huggingface.co/spaces/Wauplin/leaderboard",
+            "api": "https://huggingface.co/api/spaces/Wauplin/leaderboard",
+        },
+        "owner": {"id": "6273f303f6d63a28483fde12"},
+    },
+    "discussion": {
+        "id": "656a0dfcadba74cd5ef4545b",
+        "title": "Update space_ci/webhook.py",
+        "url": {
+            "web": "https://huggingface.co/spaces/Wauplin/leaderboard/discussions/4",
+            "api": "https://huggingface.co/api/spaces/Wauplin/leaderboard/discussions/4",
+        },
+        "status": "closed",
+        "author": {"id": "6273f303f6d63a28483fde12"},
+        "num": 4,
+        "isPullRequest": True,
+        "changes": {"base": "refs/heads/main"},
+    },
+    "webhook": {"id": "656a05348c99518820a4dd54", "version": 3},
+}
+
+
+def test_deserialize_payload_example_with_comment() -> None:
+    """Confirm that the test stub can actually be deserialized."""
+    payload = WebhookPayload.parse_obj(WEBHOOK_PAYLOAD_CREATE_DISCUSSION)
+    assert payload.event.scope == WEBHOOK_PAYLOAD_CREATE_DISCUSSION["event"]["scope"]
+    assert payload.comment is not None
+    assert payload.comment.content == "Add co2 emissions information to the model card"
+
 
-def test_deserialize_payload_example() -> None:
+def test_deserialize_payload_example_without_comment() -> None:
     """Confirm that the test stub can actually be deserialized."""
-    payload = WebhookPayload.parse_obj(WEBHOOK_PAYLOAD_EXAMPLE)
-    assert payload.event.scope == WEBHOOK_PAYLOAD_EXAMPLE["event"]["scope"]
+    payload = WebhookPayload.parse_obj(WEBHOOK_PAYLOAD_UPDATE_DISCUSSION)
+    assert payload.event.scope == WEBHOOK_PAYLOAD_UPDATE_DISCUSSION["event"]["scope"]
+    assert payload.comment is None
 
 
 @require_webhooks
@@ -145,7 +183,7 @@ class TestWebhooksServerRun(unittest.TestCase):
             # Run without blocking
             with patch.object(huggingface_hub._webhooks_server, "_is_local", False):
                 # Run without tunnel
-                self.app.run()
+                self.app.launch()
                 return TestClient(self.app.fastapi_app)
 
     def test_run_print_instructions(self):
@@ -161,7 +199,7 @@ class TestWebhooksServerRun(unittest.TestCase):
     def test_run_parse_payload(self):
         """Test that the payload is correctly parsed when running the app."""
         response = self.client.post(
-            "/webhooks/test_webhook", headers=self.HEADERS_VALID_SECRET, json=WEBHOOK_PAYLOAD_EXAMPLE
+            "/webhooks/test_webhook", headers=self.HEADERS_VALID_SECRET, json=WEBHOOK_PAYLOAD_CREATE_DISCUSSION
         )
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.json(), {"scope": "discussion"})