Skip to content

Conversation

adrinjalali
Copy link
Member

This is a handy script to remove all repos under the skops user, which is only used for CI.

cc @BenjaminBossan @merveenoyan

If all goes well, the user should be empty, as in, we should try to delete repos that we create in CI and in examples at the end.

But if there are leftovers, we can run this script every now and then.

Later we can improve this by making it remove older than X days repos and have it as a cron job on the CI.

@LysandreJik
Copy link

LysandreJik commented Jul 21, 2022

Handy script! If this script is made to be run only from time to time in order to clean things up, the token could be in secrets rather than in plaintext (always a bit uncomfortable by plaintext tokens 😄)

@LysandreJik
Copy link

Just saw https://github.com/skops-dev/skops/blob/main/skops/hub_utils/tests/common.py so I understand it would need to significantly change your setup for this to happen and work on forks

@adrinjalali
Copy link
Member Author

Yeah GH doesn't let you expose secrets the same way other platforms such as readthedocs or CircleCI do.

I played around a bit to fix the issue, but it was taking longer than I liked it to, and decided to leave it in the repo for now.

I agree in the future it'd be nice to have it fixed, and there is a way, just not a straightforward one.

Opened #47 to keep track of this.

Copy link
Collaborator

@BenjaminBossan BenjaminBossan left a comment

Choose a reason for hiding this comment

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

This should come in handy. I only have a few nits, none blocking a merge.

adrinjalali and others added 3 commits July 21, 2022 11:19
Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
Co-authored-by: Benjamin Bossan <BenjaminBossan@users.noreply.github.com>
@merveenoyan
Copy link
Collaborator

But if there are leftovers, we can run this script every now and then.

Later we can improve this by making it remove older than X days repos and have it as a cron job on the CI.

I kinda don't know why we need this given we could remove repos at the end of every test, but LGTM, looks handy

@adrinjalali
Copy link
Member Author

I kinda don't know why we need this given we could remove repos at the end of every test, but LGTM, looks handy

Yes, we try to do that, but this morning I ran the script and it removed over 100 repos.

@LysandreJik
Copy link

Indeed, we have similar occurrences in the hfh repository (but it's on staging so it's fine). This can be due to an aborted runner for example, if you stop them as soon as there's a failure, or a server error/connection error on repo deletion.

@adrinjalali
Copy link
Member Author

@merveenoyan @BenjaminBossan if nothing's left, feel free to merge :)

I've opened #49 to track the related future work on this.

@BenjaminBossan BenjaminBossan merged commit 5e02bc5 into skops-dev:main Jul 21, 2022
@adrinjalali adrinjalali deleted the clean branch July 21, 2022 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants