Spaces:

Wauplin

/

gradio-space-ci

like 6

Running

Following the creation of this PR, an ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been started. Any changes pushed to this PR will be synced with the test Space.
Since this PR has not been created by a trusted author, the ephemeral Space has not been configured with the correct hardware, storage, and secrets. An admin must configure it manually.
(This is an automated message.)

thank you AI Wauplin ✨✨

        # TODO:
        # this is limited to the owner only
        # check if it works with organizations

=> what I would do is to rely on the trusted_authors list for that. It works both for users and orgs.

trusted_authors: List[str] = EPHEMERAL_SPACES_CONFIG["trusted_authors"]

...
if(... and payload.discussion.author.id in trusted_authors)

Also important to keep in mind:

• deploying ephemeral Space always happen (no matter if trusted author or not, no matter if deploy comment or not)
• configuring ephemeral Space should only happen:
  • if PR opened by trusted author
  • OR if a trusted author write a comment with a command /trust_pr AND that comment is made after the last commit
• as soon as a new commit is pushed to the PR, the ephemeral Space should be deleted and recreated from scratch to avoid any leaks (i.e. the /trust_pr command should only be valid for the previous commits, not the following ones)

thanks for clarifying things, i'll update this pr shortly to add the necessary utilities.
also can you add me to the list of trusted users on this repo momentarily, i might need an ephemeral space on this one to debug things slowly (if the ephemeral space and the original space slash when writing on the dataset then there is no need, just wanted to highlight that beforehand )

also can you add me to the list of trusted users on this repo momentarily, i might need an ephemeral space on this one to debug things slowly

Can you try duplicating this Space and make a PR on your duplicated Space first? This shouldn't be the normal workflow but it'd be a good way to test if Space CI also works with duplicated Spaces. You can duplicate it going on https://huggingface.co/spaces/Wauplin/gradio-space-ci/discussions/7?duplicate=true + set a HF_TOKEN with your user access token. No need to add a space_ci_secret. Thanks in advance for trying it! (and if it doesn't work, I'll add you to trusted authors yes :))

@Wauplin it didn't not work, it says I don't have access to https://huggingface.co/datasets/Wauplin/gradio-space-ci-report/resolve/main/report.jsonl , could you create a temp dataset? or better yet create a temp organization and add me there (I might need to take a look at the space logs), you can make a copy of both the dataset and the space there

i can create the organization and and use my token there, problem is with the dataset, coz i have no idea what data it contains

Arf, true. You need to change this line in your duplicated Space and replace it with "not-lain/gradio-space-ci-report". And you should create this dataset repo first as well (with an empty file report.jsonl in it. Nothing really important , we don't need to share it in an org - a new one is fine.

What I want to check is if the Space CI will work in the duplicated Space. This error is more related to this Space logic itself which I don't care much to solve (it's not meant to be duplicated anyway)

@Wauplin thanks a lot for the feedback, i'll start working on it

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

cc @Wauplin for review.
if you want to try this out before merging I set up a space at https://huggingface.co/spaces/not-lain/gradio-space-ci.

how this works is that we added 2 functions set_config and unset_config and each one of them is triggered by /trust_pr or untrust_pr
If someone not in the trusted_authors updates a PR, we trigger the unset_config function by default.

the only things i did not setup yet are the recover_after_restart, and maybe an automated message after using either of these commands

ok this breaks down after a commit is made by an untrusted author, i'll fix this in a bit

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

I think we can update the default trusted list based on the _event parameter after using the get_discussion_details function.

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

@Wauplin this is ready for a review rn
I have pretty much the same code in https://huggingface.co/spaces/not-lain/gradio-space-ci if you want to try it out.
I tried my best to try every possible combination i can think of, would suggest you reach out to someone to help you out try all possible combinations (unfortunately I don't have any friends on HF so i did my best so far), and all I can say that it does what it says so far.

• when a trusted author says : /trust_pr we configure it
• when a trusted author says : /untrust_pr we reset the ephemeral space configuration
• when a new commit is uploaded we reset the ephemeral space if a trusted author does not make it

unfortunately I don't have any friends on HF so i did my best so far

My best friend on the Hub is usually me connecting with a fake account 😄💔

• when a trusted author says : /trust_pr we configure it
• when a trusted author says : /untrust_pr we reset the ephemeral space configuration
• when a new commit is uploaded we reset the ephemeral space if a trusted author does not make it

What would be the use case to use /untrust_pr exactly? When would a user want to use it? (genuinely asking as I might be missing something).

As I see it, a PR is "not trusted" by default. If a trusted author comment /trust_pr, then the PR is considered as "trusted" and the ephemeral Space can be configured. However, as soon as a new commit is pushed, it shouldn't be trusted anymore. As suggested in this comment I do think we should delete and recreate the ephemeral Space in case of new untrusted commits on the PR. So what I would do:

1. on each comment or commit, check if the ephemeral PR is in sync. If it's already in sync, then do nothing.
2. on each comment or commit, check if the PR changes should be considered as trusted. 2 rules apply here: either PR author is trusted (same as now) OR a comment /trust_pr has been added after the last commit on the PR. This can be checked by listing all commits and comments and checking their creation timestamp.
3. if the PR is trusted, sync it. If secrets/variables/hardware have to be set, set them.
4. if the PR is not trusted:
   1. if the ephemeral Space has something configured (variables/secrets/hardware), delete it + recreate
   2. if the ephemeral Space is not configured, then sync it normally

Does that make sense to you? Do you see a corner case that would not be handled by the above logic?

What I'm afraid with unsetting configuration is that:

• it might fail. If the delete_secret or delete_hardware HTTP fail for any reason, then there can be a security breach
• it might leak cached files. Let's say the Space caches a private model on a persistent storage, reloading the Space -even without secrets- would make the weights accessible in the cache.
• maybe other cases I'm not thinking about

A delete + recreate is more abrupt but at least it ensures better security. Do you see my point?

@Wauplin yessss, i'll switch to you a delete and recreate approach then🫡

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

Following new commits that happened in this PR, the ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been updated.
(This is an automated message.)

Following the creation of this PR, an ephemeral Space Wauplin/gradio-space-ci-ci-pr-7 has been started. Any changes pushed to this PR will be synced with the test Space.
Since this PR has not been created by a trusted author, the ephemeral Space has not been configured with the correct hardware, storage, and secrets. An admin must configure it manually.
(This is an automated message.)