Skip to content

Cdn assets#1265

Merged
pngwn merged 7 commits into
mainfrom
cdn-assets
May 15, 2022
Merged

Cdn assets#1265
pngwn merged 7 commits into
mainfrom
cdn-assets

Conversation

@pngwn
Copy link
Copy Markdown
Member

@pngwn pngwn commented May 15, 2022

This PR introduces a new dedicated frontend build for the CDN.

This was relatively problematic. Vite supports a custopm base URL for preloading assets and for CSS files, and normal static import x from y statements use their own url to resolve dependencies but dynamic imports (import()), which we use for code splitting the many components we have, use the requesters URL which in our case is almost always wrong when share=True. I have fixed this by patching import() to use a custom fix_import which uses import.meta.url (the url of the file itself) to resolve the correct url fopr the module. This should resolve all of our issues, I have tested it as best I can locally.

This doesn't require any backend changes, we just copy accross the cdn/index.html to frontend/share.html so we can utilise the current logic when share=True. the HTML file requests the correct assets + the CDN bundle takes care of the rest.

In order to utilise this new build we need to build twice when uploading to pypi, once for local and once for the cdn. The CDN version needs a GRADIO_VERSION env var passing. pnpm build will still build the local version but pnpm build:cdn will build the cdn version.

I have updated the upload_to_pypi script to make it work (i think).

Copy link
Copy Markdown
Contributor

@aliabid94 aliabid94 left a comment

Choose a reason for hiding this comment

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

LGTM!

@pngwn pngwn merged commit 0c94b56 into main May 15, 2022
@pngwn pngwn deleted the cdn-assets branch May 15, 2022 19:43
import nested from "tailwindcss/nesting/index.js";

const GRADIO_VERSION = process.env.GRADIO_VERSION;
const CDN_URL = `https://gradio.s3-us-west-2.amazonaws.com/${GRADIO_VERSION}/`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FWIW S3 is not really a CDN ie. is pretty slow especially when you're far from the region (here us-west) but it'd be pretty trivial to hook a Cloudfront distribution in front of it if needed to speed things up @pngwn

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree. After 3.0 I'd like to rethink our CDN stuff. It would be cool to have a nice cdn.gradio.app with / resolving to the js entrypoint (which is currently broken). Bare url would redirect to latest. cdn.gradio.app/3/ redirecting to latest 3.x, cdn.gradio.app/3.1/ redirecting to latest 3.1.x etc. For users embedding gradio via CDN, needing to manually track versions is frustrating and people will get out of date quickly.

But I also want to change how we do releases (+ asset deployments). I'd like us to have everything in CI with the asset deployments IaCed with TF or something (i assume we use TF here but havent checked).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Use of the phrase 'CDN' is very flexible here. :D But using a real CDN is the aim.

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.

3 participants