Basic site logos should hit the CDN

When we have a CDN defined, we should be leveraging that for various image assets (e.g. logo url, logo small url) that are relative URLs

e.g.: logo_url='/uploads/default/original/1X/5b5cc13aa92459bb084ca1485bca80c480bfa6f0.gif'

3 Likes

Where do you see that exactly?

sjcthree for example - the site logos were uploaded during the wizard - I haven’t changed them.

3 Likes

@nbianca can you make sure the images in the site settings use the CDN whenever they’re pointing at local images?

2 Likes

I tried reproducing this with latest-version and v1.9.0 (had to cherry-pick a6c589d8). My CDN was provided by a local Minio instance.

The code also looks fine as it uses the same logic for other uploads.

@supermathie, have you seen it reproducing recently?

4 Likes

I haven’t set up any new sites recently, but I’ll re-run the wizard on a site and try it.

3 Likes

I just tried this with latest discourse version on a new site.

image uploads for logos only use the s3 urls instead of given s3_cdn url. (but S3 cdn url is correctly picked up in image uploads within a post)

Also, discourse used to have a text box under the logo images where we can fill a custom image link. That was quite useful. Even in this case, we could have manually updated the CDN url if there is a text box.

1 Like

See Downsides of the new logo UI in site settings

4 Likes

@hawk thanks for the note. I am part of the conversations there.

I am actually looking for a fix / way to get the s3_cdn url in logos. Having a text box, is just a temporary fix.

2 Likes

Thank you for your report, @gingerman. I had another look and found the culprit.

I submitted a pull request to fix the issue and the changes will be merged soon.

https://github.com/discourse/discourse/pull/6698

6 Likes