ref: allow hosted js sdk bundles

[aldy505]

Since this is hanging on my branch, I guess I'll make it out here. A part of making the JS SDK loader script available on the self-hosted sentry. Refer to getsentry/sentry#22715.

Since not everyone is happy on hosting the JS SDK themself, therefore I made it into a feature flag. Set SETUP_JS_SDK_ASSETS=1 on the .env file to enable the script. Also set SETUP_JS_SDK_KEEP_OLD_ASSETS=1 to never delete (or basically cleanup) previous JS SDK version.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

[BYK]

Wow, this looks quite nice! I'd make it opt-out rather than opt-in as it the assets shouldn't take too much space and this is a great quality of life improvement.

I still don't get the reasoning behind using an external volume compared to a project-specific volume tho. Can you expand on that a bit?

[aldy505]

Wow, this looks quite nice! I'd make it opt-out rather than opt-in as it the assets shouldn't take too much space and this is a great quality of life improvement.

I still don't get the reasoning behind using an external volume compared to a project-specific volume tho. Can you expand on that a bit?

@BYK I made it quite a while ago so I don't really remember. Probably because the nginx container has web container as the dependency, so I think we shouldn't start the nginx container at all.

If there is something like "docker compose volume create", that'd be nice.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.01%. Comparing base (036f6d4) to head (7bed4a8).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3365   +/-   ##
=======================================
  Coverage   99.01%   99.01%           
=======================================
  Files           3        3           
  Lines         203      203           
=======================================
  Hits          201      201           
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BYK]

@BYK I made it quite a while ago so I don't really remember. Probably because the nginx container has web container as the dependency, so I think we shouldn't start the nginx container at all.

I mean you don't need to spin up nginx (and its dependencies) to be able to use the volume? We can bind that to a different service or just use --no-deps to not start any other dependencies. You may add a pseudo service with a base Debian image that defines all volumes on it and use that as the "install helper". We can add that to a profile such as installer so it is never included in regular setups.

WDYT?

[aldy505]

@BYK I made it quite a while ago so I don't really remember. Probably because the nginx container has web container as the dependency, so I think we shouldn't start the nginx container at all.

I mean you don't need to spin up nginx (and its dependencies) to be able to use the volume? We can bind that to a different service or just use --no-deps to not start any other dependencies. You may add a pseudo service with a base Debian image that defines all volumes on it and use that as the "install helper". We can add that to a profile such as installer so it is never included in regular setups.

WDYT?

In the end, I used --no-deps.

But I'd still prefer making this opt-in instead. I don't know about other people though.

[aldy505]

Finally.

The test passed, but I still need to modify the wordings & docs for this.

[hubertdeng123]

Looks pretty good, just one minor comment. This is awesome work!

[hubertdeng123]

IMO if we're going to include this as part of the install script moving forwards, I'd like to see it in the .env file just to surface this as something people can set up. We can leave it as default for opt-in

[aldy505]

Oh not you too :(

[aldy505]

Oh sorry I read that wrong. Lol.

[aldy505]

Okay done