Skip to content

Upload all demos to spaces#2281

Merged
aliabd merged 26 commits into
mainfrom
aliabd/demos-to-spaces
Oct 14, 2022
Merged

Upload all demos to spaces#2281
aliabd merged 26 commits into
mainfrom
aliabd/demos-to-spaces

Conversation

@aliabd
Copy link
Copy Markdown
Contributor

@aliabd aliabd commented Sep 16, 2022

This PR:

  • uploads all 164 demos to spaces, and loads the ones the website references from there.
  • makes the docs track pip instead of main. Both the website and the demos will only update when the gradio version changes instead of on every push to main
  • greatly simplifies the website infra by entirely removing the demos docker container. Also makes it easy to reference more demos elsewhere.

You can play around with the changes on the devserver.

Fixes #2277

@github-actions
Copy link
Copy Markdown
Contributor

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2281-all-demos

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Sep 16, 2022

Given that it takes >15min to update all the spaces, and they're only using the latest pip version of gradio (not building from git) I'm now wondering if it makes more sense to update only when the version changes, or if anything in /demos changes. Thoughts?

Also, even with Demos, we still only embed the minority of demos in /demos on the website (tough sentence to parse I know...) So maybe uploading everything isn't the right way to go. Me and @aliabid94 will go through all demos at some point and consolidate them a bit, remove duplicates, etc

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Sep 16, 2022

I'm not sure if its the sheer number of them, but on a few demos Spaces will error out during the build without any actual error. A factory reboot fixes of the space fixes it. Maybe its a timeout thing

@abidlabs
Copy link
Copy Markdown
Member

Great points @aliabd. My thoughts:

Given that it takes >15min to update all the spaces, and they're only using the latest pip version of gradio (not building from git) I'm now wondering if it makes more sense to update only when the version changes, or if anything in /demos changes. Thoughts?

I definitely agree. In addition, if a specific demo in /demos changes, ideally we would only trigger a rebuild for that demo

Also, even with Demos, we still only embed the minority of demos in /demos on the website (tough sentence to parse I know...) So maybe uploading everything isn't the right way to go. Me and @aliabid94 will go through all demos at some point and consolidate them a bit, remove duplicates, etc

Yes, we've definitely had a lot of "demo creep". Let's only upload the demos that are being somewhere on the website, which might help with the next point as well...

I'm not sure if its the sheer number of them, but on a few demos Spaces will error out during the build without any actual error. A factory reboot fixes of the space fixes it. Maybe its a timeout thing

Good catch. I think we should let the Spaces team know in case it's some infra thing. We should be careful here because we definitely don't want any of the embedded demos on the homepage to go down, or really any of the embedded demos. Does it make sense to maybe trigger a job 15 min after the demos get refreshed that pings all of the Spaces URLs and confirms that they are up?

@freddyaboulton
Copy link
Copy Markdown
Collaborator

@aliabd How come the demos use the latest published version of gradio on pypi as opposed to the latest commit to main? I'm worried about the discrepancy caused by the docs showing main but the demos showing the "stable" version, i.e. latest pypi version.

It's on our roadmap to toggle between latest and stable (#1650) but until we do that everything on the website should be the same version in my opinion.

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Sep 16, 2022

@abidlabs I will play around with this for a bit and find the right set up but in my opinion:

  • Every demo (that we decide should remain) in /demo should be up on spaces. It's the same value prop as keeping it in the repo, except that now users can also play around with them on hf.co/spaces
  • Definitely agree we should only rebuild a specific demo if it changes, that should resolve all of these issues (we will only see bad upload times or potential spaces infra issues with pip releases if we continue to track stable)
  • I'll set up some URL checker if the problem persists.

@freddyaboulton I honestly am not sure why Docs track latest from main and not stable from pip. We should at some point allow a toggle but if we have to choose it should definitely be stable in my opinion, given that that's what the overwhelming majority of users will build with. Let's move this discussion to slack but great point on avoiding discrepancies.

@aliabid94
Copy link
Copy Markdown
Contributor

where did you move this discussion to slack? (I think this is a fine place for this discussion).
If we want the docs to track stable rather than main (which I agree with), then we should have the entire website track stable as well. Otherwise there will be conflicts that will break the website (such as if someone pushes changes to the documentation format in the gradio library and the website documentation.)

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Sep 19, 2022

Yeah good point @aliabid94, but if the entire website tracks stable how do we make small changes unrelated to the library? Do we have to upgrade pip in that case? I guess looking backwards we don't make that many changes anymore because things have stabilized a bit. So I'm okay with that.

@aliabd aliabd marked this pull request as draft September 22, 2022 17:58
@aliabd aliabd marked this pull request as ready for review October 12, 2022 21:53
@@ -67,10 +64,11 @@ def upload_demo_to_space(
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: you don't have to rename run.py to app.py and could instead change app_file to run.py

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks @abidlabs, I set the app_file to run.py now. I also updated all the spaces to remove the app.py file and reuploaded with run.py

@@ -1,20 +1,48 @@
<h1 id="upcoming-release">Upcoming Release</h1>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This file could be git-ignored

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Properly git ignored it, thanks!

Copy link
Copy Markdown
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

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

Amazing job @aliabd! LGTM

@aliabid94
Copy link
Copy Markdown
Contributor

There is the possibility of a race condition here - the website restarts on a new version.txt change and launches spaces with this new version, but the new version isn't on pypi yet.

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Oct 14, 2022

Very good catch @aliabid94

Just added check_version.py which pings the pypi url to see if the version is up every minute for 10 minutes (it takes 3-4 minutes after commit for the version to be up on pypi). If it's up continues to build the website, upload spaces, etc using it. If it isn't up after 10 minutes, the python script exits with an error code that is passed on to the slack notification.

What do you think?

Comment thread website/reload_website.sh
git pull > /tmp/git_changes.txt

if grep -q "Already up to date." /tmp/git_changes.txt; then
if ! grep -q "gradio/version.txt" /tmp/git_changes.txt; then
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this running in a cron job? How often does it run?

I think this logic makes sense. The version.txt should only change when an official release goes out. What happens if we accidentally change version.txt to be a pre-release version, like 3.4.1b10? Does check_version error out?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah this runs in a cronjob every minute.

Yes, python check_version.py errors out if it can't find the version on pypi. (if the url f"https://pypi.org/project/gradio/{VERSION}/" doesn't return 200)

In that case the web tracker bot will send a notification to slack that looks like this:

Could not find gradio v3.4.1b10 on pypi: https://pypi.org/project/gradio/3.4.1b10/ does not exist

A bit confused though, is this something we do? change version.txt without intending it to upload to pypi?

demos = [demo for demo in demos if demo != "all_demos" and os.path.isdir(os.path.join(GRADIO_DEMO_DIR, demo)) and os.path.exists(os.path.join(GRADIO_DEMO_DIR, demo, "run.py"))]

if __name__ == "__main__":
if AUTH_TOKEN is not None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, can finally build locally again!

@aliabid94
Copy link
Copy Markdown
Contributor

LGTM

Copy link
Copy Markdown
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thanks for the awesome PR @aliabd ! Nothing more to comment except what we discussed at our huddle.

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Oct 14, 2022

Thanks @freddyaboulton @abidlabs for the feedback and catching the prereleases problem.

To avoid reflecting prereleases on the docs, I now check to make sure that the version in version.txt is the same as the latest version on pypi (which skips prereleases).

So now check_version will exit and halt the build on two scenarios:

  • The version doesn't exist on pypi due to a failed upload or mistake. We wait up to 10 min before halting and this error will be piped to slack:
Could not find gradio v{X.X.X} on pypi: https://pypi.org/project/gradio/{X.X.X}/ does not exist
  • The version exists on pypi, but is not the latest version listed. We don't wait at all for this and halt as soon as the check fails. This error will be piped to slack:
Did not restart: gradio v{X.X.X} is a prelease, or a later version exists.

What do you think?

@abidlabs
Copy link
Copy Markdown
Member

👍 👍

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.

Move all demos to spaces and clean up

4 participants