refactor code and fix readme title

src/gradio_space_ci/webhook.py

@@ -207,13 +207,13 @@ def recover_after_restart(space_id: str) -> None:
             if not is_pr_synced(space_id=space_id, pr_num=discussion.num):
                 # Found a PR that is not yet synced
                 print(f"Recovery. Found an open PR that is not synced: {discussion.url}. Syncing it.")
-                background_pool.submit(sync_ci_space, space_id=space_id, pr_num=discussion.num)
+                background_pool.submit(sync_ci_space, space_id=space_id, pr_num=discussion.num, skip_config=False)
         if discussion.status == "merged" or discussion.status == "closed":
             ci_space_id = _get_ci_space_id(space_id=space_id, pr_num=discussion.num)
             if repo_exists(repo_id=ci_space_id, repo_type="space"):
                 # Found a PR for which the CI space has not been deleted
                 print(f"Recovery. Found a closed PR with an active CI space: {discussion.url}. Deleting it.")
-                background_pool.submit(delete_ci_space, space_id=space_id, pr_num=discussion.num)
+                background_pool.submit(delete_ci_space, space_id=space_id, pr_num=discussion.num, notify=True)
 
 
 ###
@@ -258,13 +258,13 @@ async def trigger_ci_on_pr(payload: WebhookPayload, task_queue: BackgroundTasks)
         and payload.discussion.isPullRequest
         and payload.discussion.status in ACTIVE_PR_STATUS
     ):
-        # A comment, is it by a command ?
+        # A comment, is it a command ?
         if payload.event.scope == "discussion.comment":
             handle_command(space_id=space_id, payload=payload)
         # Always sync (in case the space was sleeping or building)
         if not is_pr_synced(space_id=space_id, pr_num=payload.discussion.num):
             # New PR! Sync task scheduled
-            task_queue.add_task(sync_ci_space, space_id=space_id, pr_num=payload.discussion.num)
+            task_queue.add_task(sync_ci_space, space_id=space_id, pr_num=payload.discussion.num, skip_config=False)
             has_task = True
     elif (
         # Means "a PR has been merged or closed"
@@ -274,11 +274,7 @@ async def trigger_ci_on_pr(payload: WebhookPayload, task_queue: BackgroundTasks)
         and payload.discussion.isPullRequest
         and (payload.discussion.status == "merged" or payload.discussion.status == "closed")
     ):
-        task_queue.add_task(
-            delete_ci_space,
-            space_id=space_id,
-            pr_num=payload.discussion.num,
-        )
+        task_queue.add_task(delete_ci_space, space_id=space_id, pr_num=payload.discussion.num, notify=True)
         has_task = True
     elif (
         # Means "some content has been pushed to the Space" (any branch)
@@ -290,7 +286,7 @@ async def trigger_ci_on_pr(payload: WebhookPayload, task_queue: BackgroundTasks)
             if discussion.is_pull_request and discussion.status in ACTIVE_PR_STATUS:
                 if not is_pr_synced(space_id=space_id, pr_num=discussion.num):
                     # Found a PR that is not yet synced
-                    task_queue.add_task(sync_ci_space, space_id=space_id, pr_num=discussion.num)
+                    task_queue.add_task(sync_ci_space, space_id=space_id, pr_num=discussion.num, skip_config=False)
                     has_task = True
 
     if has_task:
@@ -321,9 +317,9 @@ def is_pr_synced(space_id: str, pr_num: int) -> bool:
     return last_synced_sha == last_pr_sha
 
 
-def sync_ci_space(space_id: str, pr_num: int) -> None:
+def sync_ci_space(space_id: str, pr_num: int, skip_config: bool = False) -> None:
     print(f"New task: sync ephemeral env for {space_id} (PR {pr_num})")
-    if is_pr_synced(space_id=space_id, pr_num=pr_num):
+    if is_pr_synced(space_id=space_id, pr_num=pr_num) and not skip_config:
         print("Already synced. Nothing to do.")
         return
 
@@ -334,11 +330,13 @@ def sync_ci_space(space_id: str, pr_num: int) -> None:
 
     # Configure ephemeral Space if trusted author
     is_configured = False
-    if is_new:
+    if not skip_config and is_new:
         is_configured = configure_ephemeral_space(space_id=space_id, pr_num=pr_num)
 
     # Download space codebase from PR revision
-    snapshot_path = Path(snapshot_download(repo_id=space_id, revision=f"refs/pr/{pr_num}", repo_type="space"))
+    snapshot_path = Path(
+        snapshot_download(repo_id=space_id, revision=f"refs/pr/{pr_num}", repo_type="space", force_download=True)
+    )
 
     # Overwrite README file in cache (/!\)
     readme_path = snapshot_path / "README.md"
@@ -360,7 +358,9 @@ def sync_ci_space(space_id: str, pr_num: int) -> None:
     readme_path.unlink(missing_ok=True)
 
     # Post a comment on the PR
-    if is_new and is_configured:
+    if is_new and skip_config:
+        notify_pr(space_id=space_id, pr_num=pr_num, action="untrusted_pr")
+    elif is_new and is_configured:
         notify_pr(space_id=space_id, pr_num=pr_num, action="created_and_configured")
     elif is_new:
         notify_pr(space_id=space_id, pr_num=pr_num, action="created_not_configured")
@@ -390,7 +390,14 @@ def create_ephemeral_space(space_id: str, pr_num: int) -> bool:
             raise
 
 
-def configure_ephemeral_space(space_id: str, pr_num: int) -> bool:
+##
+# configure_ephemeral_space logic
+# if a pr is made by a trusted author or an author trusts a pr
+# => we configure the ephemeral space
+##
+
+
+def configure_ephemeral_space(space_id: str, pr_num: int, trusted_pr=False) -> bool:
     # Config values
     ci_space_id = _get_ci_space_id(space_id=space_id, pr_num=pr_num)
     trusted_authors: List[str] = EPHEMERAL_SPACES_CONFIG["trusted_authors"]
@@ -401,25 +408,25 @@ def configure_ephemeral_space(space_id: str, pr_num: int) -> bool:
 
     # Check if trusted author
     details = get_discussion_details(repo_id=space_id, repo_type="space", discussion_num=pr_num)
-    if details.author not in trusted_authors:
-        return False  # not a trusted author => do NOT set secrets, hardware, storage, etc.
-
-    # Configure space
-    for key, value in variables.items():
-        add_space_variable(ci_space_id, key, value)
-    for key, value in secrets.items():
-        add_space_secret(ci_space_id, key, value)
-
-    # Request hardware/storage for space
-    if hardware is not None and hardware != SpaceHardware.CPU_BASIC:
-        request_space_hardware(ci_space_id, hardware, sleep_time=5 * 60)  # sleep after 5min on PR Spaces with GPU
-    if storage is not None:
-        request_space_storage(ci_space_id, storage)
+    if details.author in trusted_authors or trusted_pr is True:
+        # Configure space
+        for key, value in variables.items():
+            add_space_variable(ci_space_id, key, value)
+        for key, value in secrets.items():
+            add_space_secret(ci_space_id, key, value)
+
+        # Request hardware/storage for space
+        if hardware is not None and hardware != SpaceHardware.CPU_BASIC:
+            request_space_hardware(ci_space_id, hardware, sleep_time=5 * 60)  # sleep after 5min on PR Spaces with GPU
+        if storage is not None:
+            request_space_storage(ci_space_id, storage)
 
-    return True
+        return True
+    else:
+        return False
 
 
-def delete_ci_space(space_id: str, pr_num: int) -> None:
+def delete_ci_space(space_id: str, pr_num: int, notify: bool = True) -> None:
     print(f"New task: delete ephemeral env for {space_id} (PR {pr_num})")
 
     # Delete
@@ -431,7 +438,9 @@ def delete_ci_space(space_id: str, pr_num: int) -> None:
         return
 
     # Notify about deletion
-    notify_pr(space_id=space_id, pr_num=pr_num, action="deleted")
+    if notify is True:
+        # This logic is adaped across multiple functions so we will not always notify the pr
+        notify_pr(space_id=space_id, pr_num=pr_num, action="deleted")
 
 
 def notify_pr(
@@ -451,9 +460,9 @@ def notify_pr(
     elif action == "deleted":
         comment = NOTIFICATION_TEMPLATE_DELETED
     elif action == "trusted_pr":
-        comment = NOTIFICATION_TEMPLATE_TRUSTED_PR
+        comment = NOTIFICATION_TEMPLATE_TRUSTED_PR.format(ci_space_id=ci_space_id)
     elif action == "untrusted_pr":
-        comment = NOTIFICATION_TEMPLATE_UNTRUSTED_PR
+        comment = NOTIFICATION_TEMPLATE_UNTRUSTED_PR.format(ci_space_id=ci_space_id)
     else:
         raise ValueError(f"Status {action} not handled.")
 
@@ -464,57 +473,13 @@ def _get_ci_space_id(space_id: str, pr_num: int) -> str:
     return f"{space_id}-ci-pr-{pr_num}"
 
 
-def set_config(space_id: str, pr_num: str) -> None:
-    """a function to set the ephemerial space config"""
-    variables: Dict[str, str] = EPHEMERAL_SPACES_CONFIG["variables"]
-    secrets: Dict[str, str] = EPHEMERAL_SPACES_CONFIG["secrets"]
-    hardware: Optional[SpaceHardware] = EPHEMERAL_SPACES_CONFIG["hardware"]
-    storage: Optional[SpaceHardware] = EPHEMERAL_SPACES_CONFIG["storage"]
-    ci_space_id = _get_ci_space_id(space_id=space_id, pr_num=pr_num)
-    # Configure space
-    for key, value in variables.items():
-        add_space_variable(ci_space_id, key, value)
-    for key, value in secrets.items():
-        add_space_secret(ci_space_id, key, value)
-
-    # Request hardware/storage for space
-    if hardware is not None and hardware != SpaceHardware.CPU_BASIC:
-        request_space_hardware(ci_space_id, hardware, sleep_time=5 * 60)  # sleep after 5min on PR Spaces with GPU
-    if storage is not None:
-        request_space_storage(ci_space_id, storage)
-
-
 def rebuild_space(space_id: str, pr_num: int) -> None:
     "a function to rebuild the ephemeral space without config"
     # This is useful to cut down on resource usage and to remove tokens from
     # the ephemeral space
-    ci_space_id = _get_ci_space_id(space_id=space_id, pr_num=pr_num)
-    try:
-        delete_repo(repo_id=ci_space_id, repo_type="space")
-    except RepositoryNotFoundError:
-        pass
-    create_ephemeral_space(space_id=space_id, pr_num=pr_num)
-    # Download space codebase from PR revision
-    snapshot_path = Path(snapshot_download(repo_id=space_id, revision=f"refs/pr/{pr_num}", repo_type="space"))
-
-    # Overwrite README file in cache (/!\)
-    readme_path = snapshot_path / "README.md"
-    card = RepoCard.load(readme_path)
-    setattr(card.data, "synced_sha", snapshot_path.name)  # latest sha
-    card.data.title = f"{card.data.title} (ephemeral #{pr_num})"
-    card.save(readme_path)
-
-    # Sync space codebase with PR revision
-    upload_folder(
-        repo_id=ci_space_id,
-        repo_type="space",
-        commit_message=f"Sync CI Space with PR {pr_num}.",
-        folder_path=snapshot_path,
-        delete_patterns="*",
-    )
-
-    # Delete readme file from cache (just in case)
-    readme_path.unlink(missing_ok=True)
+    delete_ci_space(space_id=space_id, pr_num=pr_num, notify=False)
+    # create a new synced ephemeral space
+    sync_ci_space(space_id=space_id, pr_num=pr_num, skip_config=True)
 
 
 def handle_modification(space_id: str, discussion: Any) -> None:
@@ -522,10 +487,13 @@ def handle_modification(space_id: str, discussion: Any) -> None:
     if not repo_exists(ci_space_id):
         return
     details = get_discussion_details(repo_id=space_id, repo_type="space", discussion_num=discussion.num)
-    event_author = details.events[-1]._event["author"]["name"]  # username of that event
-    if event_author not in EPHEMERAL_SPACES_CONFIG["trusted_authors"]:
-        # Untrusted author, we rebuild the space
-        rebuild_space(space_id=space_id, pr_num=discussion.num)
+    # If last commit is not by a trusted author we rebuild it
+    for event in details.events[::-1]:
+        if event.type == "commit":
+            if event._event["author"]["name"] not in EPHEMERAL_SPACES_CONFIG["trusted_authors"]:
+                rebuild_space(space_id=space_id, pr_num=discussion.num)
+            else:
+                return
 
 
 def handle_command(space_id: str, payload: WebhookPayload) -> None:
@@ -535,11 +503,10 @@ def handle_command(space_id: str, payload: WebhookPayload) -> None:
     event_author = details.events[-1]._event["author"]["name"]  # username of that event
     if event_author in EPHEMERAL_SPACES_CONFIG["trusted_authors"]:
         if payload.comment.content == "/trust_pr":
-            set_config(space_id=space_id, pr_num=pr_num)
+            configure_ephemeral_space(space_id=space_id, pr_num=pr_num, trusted_pr=True)
             notify_pr(space_id=space_id, pr_num=pr_num, action="trusted_pr")
         elif payload.comment.content == "/untrust_pr":
             rebuild_space(space_id=space_id, pr_num=pr_num)
-            notify_pr(space_id=space_id, pr_num=pr_num, action="untrusted_pr")
 
 
 NOTIFICATION_TEMPLATE_CREATED_AND_CONFIGURED = """\
@@ -566,12 +533,14 @@ _(This is an automated message.)_
 
 NOTIFICATION_TEMPLATE_TRUSTED_PR = """\
 This PR has been granted temporary trust status Thus granting it with the appropriate approriate hardware, storage, and secrets.
-Trust status will be revokeduser either when a trusted author uses `/untrust_pr` command or when new commits are pushed to this PR.
+You can access the ephemeral Space at [{ci_space_id}](https://huggingface.co/spaces/{ci_space_id}).
+Trust status will be revoked either when a trusted author uses `/untrust_pr` command or when new commits are pushed to this PR.
 _(This is an automated message.)_
 """
 
 NOTIFICATION_TEMPLATE_UNTRUSTED_PR = """\
 This PR has been untrusted. Thus resetting all hardware, storage, and secrets.
+You can access this PR's ephemeral Space at [{ci_space_id}](https://huggingface.co/spaces/{ci_space_id}).
 _(This is an automated message.)_
 """