huggingface / huggingface_hub

The official Python client for the Huggingface Hub.
https://huggingface.co/docs/huggingface_hub
Apache License 2.0
2.01k stars 531 forks source link

Support COEP/COOP headers on Spaces so that WebML spaces can use multiple threads #1525

Closed josephrocca closed 1 year ago

josephrocca commented 1 year ago

Is your feature request related to a problem? Please describe. It's not possible to use multiple threads without COOP/COEP headers. Github Pages has long had this issue, and they don't seem to care, but with Hugging Face Spaces it's even more important, because WebML needs threads to get decent performance in most cases.

Describe the solution you'd like These are the headers that need to be optionally-enabled to support SharedArrayBuffers:

Cross-Origin-Embedder-Policy: credentialless
Cross-Origin-Opener-Policy: same-origin

Describe alternatives you've considered A hacky workaround: https://github.com/orgs/community/discussions/13309#discussioncomment-3844940

Additional context

josephrocca commented 1 year ago

A very simple/narrow implementation of this probably makes sense for now, but eventually it may make sense to have a more fully-fledged system akin to Netlify's: https://docs.netlify.com/routing/headers/#syntax-for-the-netlify-configuration-file

radames commented 1 year ago

Hi @josephrocca, we've contemplated this, but implementing it could break many Spaces. This is because web apps that communicate with other domains would also need to abide by the COOP/COEP rules.

I can provide an example of a SharedArrayBuffers experience on Spaces. Please note, this will only function outside of our main hub. The example is a Dockerfile container with a custom Nginx configuration.

no SharedArrayBuffer: https://huggingface.co/spaces/radames/whisper.wasm with SharedArrayBuffer: https://radames-whisper-wasm.hf.space/

josephrocca commented 1 year ago

Hey @radames yep you definitely wouldn't want to force COOP/COEP on all spaces[0] - you'd need to make it a configurable option. Currently I think the service worker hack that I linked in my original post is probably the easiest workaround for most cases, since it "just works" with the main downside being the extra page refresh that it causes when you view the page for the first time.

By the sounds of it, there are some troubles with your CDN that make it hard to add an option?

[0] Though note that the newer credentialless value for COEP (currently not available in Firefox or Safari) is less of a breaking change - but I still wouldn't force it on all spaces.

julien-c commented 1 year ago

where would you set that option? would README metadata work?

josephrocca commented 1 year ago

@xenova I think you've used Spaces more than me - any thoughts on where it might make most sense to declare headers/options like this? I do know that Netlify uses a netlify.toml file for this sort of thing, but I don't know whether or not that's a good approach here. README metadata sounds reasonable to me.

xenova commented 1 year ago

Adding this metadata to the README definitely sounds like the easiest solution to me. The docs already list quite a few items, so it probably will be okay to add something like a "headers" section (and then expose certain ones that the user is allowed to change).

radames commented 1 year ago

Hi @josephrocca the option on README.md is a good idea indeed. In that scenario I don't think we'll need the service worker hack since we would be able the provide the correct headers. The service worker hack works as well on Spaces to enable our static SDK without COEP/COOP headers.

testing with your clip-image-sorter iframe no sharedarray: https://huggingface.co/spaces/radames/test-coop-coep with sharedarray: https://radames-test-coop-coep.hf.space/index.html

julien-c commented 1 year ago

linking to a 2022 internal issue about this

@xenova would you want to add a way to control those custom headers from Space metadata? would require some changes on our internal repos moon-landing and SAM, we can coordinate on Slack if you'd like to look into it 🔥

cc @krampstudio too who mentioned he knows a bit about COEP/COOP and also cc @XciD for visibility

julien-c commented 1 year ago

And thanks for opening this issue @josephrocca you motivated me by saying we can do better than Github Pages hehe:)

xenova commented 1 year ago

@xenova would you want to add a way to control those custom headers from Space metadata?

I would say this is definitely the simplest from the user's perspective. So, if this is a possibility, I would advocate for this! That said, I'm not too sure how many different headers we'll support... and since this can grow, perhaps it would be best to use a separate configuration file (as done here).

we can coordinate on Slack if you'd like to look into it 🔥

Sounds good!

julien-c commented 1 year ago

@josephrocca and all:

THE RACE IS ON!!

image
xenova commented 1 year ago

@josephrocca and all:

THE RACE IS ON!!

image

Don't worry, we'll beat them 😏

josephrocca commented 1 year ago

Yeah I called them out a bit a couple of days ago regarding a separate feature - subdomains - that Hugging Face already has.

I'm kind of guessing that this issue will go unnoticed. Radio silence on the most upvoted (as of writing) issue of all time in the Pages category (which I posted over a year ago): COOP/COEP - https://github.com/orgs/community/discussions/13309 [...] Is anyone in charge of Pages right now?

And got that COOP/COEP reply, and also this reply on subdomains:

Unfortunately we have no plans to implement it in the short term.

Which kind of blows my mind tbh - I think this is significantly more important than COOP/COEP stuff, and had just kind of assumed that they would be working on this and was just probing for an ETA. But then again, this is all kind of expected in a big, ossified org like Github 🤷

Luckily HF is here to shake things up a bit :sunglasses:

josephrocca commented 1 year ago

Cloudflare's implementation of custom headers for static sites - as some extra design inspiration: https://developers.cloudflare.com/pages/platform/headers/

Looks like Netlify also supports a _headers file with similar (same?) syntax to Cloudflare as an alternative to their netlify.toml approach. Cloudflare has a ! operator which allows you to remove a header which has been added by a broader/less-specific rule.

radames commented 1 year ago

Hi @josephrocca , we now support COEP/COOP Headers on Spaces SDK Static thanks to @xenova!

I'll be happy to move this early test to your account on HF. https://huggingface.co/spaces/radames/OpenAI-CLIP-JavaScript

# README.md
custom_headers:
  cross-origin-embedder-policy: require-corp
  cross-origin-opener-policy: same-origin
  cross-origin-resource-policy: cross-origin

docs: https://huggingface.co/docs/hub/spaces-config-reference#:~:text=can%20be%20embedded.-,custom_headers,-%3A%20Dict%5Bstring

julien-c commented 1 year ago

if anyone has other demos leveraging this, please share them, i think this is very cool!

josephrocca commented 1 year ago

Will definitely be using HF spaces for future demos! Especially since I recently realised you can link to a foo.hf.space version that doesn't include the header, so you get the full page for your site, which was my only remaining concern (nothing wrong with the header, but having a fullscreen option is important).