Skip to content

Conversation

@raulrpearson
Copy link
Contributor

Overview

This PR refactors environment variables to allow configuration of alternative S3-compatible services for presentation storage through the config offered by ExAws.S3. It also adds get_slide_urls/1 and get_slide_urls/2 functions to the Claper.Presentations context module to encapsulate the logic to produce these URL depending on storage config. This results in small refactors of the "manage" and "presenter" live views.

I was not able to set up Minio locally successfully but was able to make this work against a Cloudflare R2 bucket. Cloudflare offers a public URL that's different from the S3-compatible endpoint, which is why I also added the S3_PUBLIC_URL env var. If not supplied, the code is intended to default to either the regular AWS or the supplied S3_SCHEME, S3_HOST and S3_PORT URL.

The PR also includes a commit that deletes some dead code: we're not using the direct upload to S3 in app.js at the moment and we're also not using the code in lib/utils. Mogrify also goes as it was only being used in one of these deleted modules. We can revert some of the deletions if you feel strongly about keeping some of this code around.

Open questions

  • I don't know much Kubernetes so not sure what to do about the code in the charts/ folder. I suppose the new variables should also be added to the template config YAML file? Do we still need this code or can we delete it?

  • I attempted to add a test to the new get_slide_urls/n functions, but I had to end up messing around with Application.put_env/4 in a way that seemed sketchy so I gave up. I was also not feeling like the tests were adding much. Let me know if you'd prefer that I add some tests.

  • Would you like me to add a related PR to the docs site explaining the new S3 config affordances?

  • When I test locally, by adding an event with a presentation, I get a duplicate event ID error like the one we're targeting in Improve event live listing #185 when the presentation is done converting/uploading. I don't think this is caused by this PR. My suggestion is that we look into fixing that in a different PR (or maybe in Improve event live listing #185) to avoid increasing the scope of this one. Just wanted to flag this in case you want to take a look.

Testing

I tested by configuring an R2 bucket and setting my env vars to the right values. If you can test against a local Minio or similar, that'd be great so that we are more certain the current design works across services. mix test still also runs successfully.

@alxlion
Copy link
Contributor

alxlion commented Dec 4, 2025

I tested with a local Minio and it uploaded well, but I cannot see the images of the presentation file in the manager. Did you manage to view the them once it had been uploaded to R2 ?

@raulrpearson raulrpearson force-pushed the allow-generic-s3-endpoints branch from 279adf0 to 0f809a5 Compare December 4, 2025 23:03
@raulrpearson
Copy link
Contributor Author

@alxlion, yes, retrieving slides from the bucket also worked. In the case of R2 I needed to provide a public URL for reads that is different from the URL for S3 operations, which is why I added the S3_PUBLIC_URL variable.

I think the problem might be that I forgot to append the port when building that URL for reads:

s3_public_url =
  get_var_from_path_or_env(
    config_dir,
    "S3_PUBLIC_URL",
    if(s3_scheme && s3_host,
      do: s3_scheme <> s3_host,
      else: "https://#{s3_bucket}.s3.#{s3_region}.amazonaws.com"
    )
  )

I'll try to push fix tomorrow.

@raulrpearson raulrpearson force-pushed the allow-generic-s3-endpoints branch from 0f809a5 to 4969b87 Compare December 5, 2025 16:23
@raulrpearson raulrpearson force-pushed the allow-generic-s3-endpoints branch from 4969b87 to 5fa7af1 Compare December 5, 2025 16:24
@raulrpearson
Copy link
Contributor Author

@alxlion, can you try again? I think it's fixed, I tried with some dummy values locally and the URL generation seems to be working as it should. Specifically the :claper, :presentations, :s3_public_url is the one that will be used to build slide jpeg URLs to display on the frontend. The value is generated 3 different ways:

  • If S3_PUBLIC_URL exists, that's the one used.
  • If S3_SCHEME, S3_HOST and optionally S3_PORT exist, the concatenation of these is used.
  • Finally, if none of the above are true, then the "https://#{s3_bucket}.s3.#{s3_region}.amazonaws.com" pattern is used.

@alxlion
Copy link
Contributor

alxlion commented Dec 6, 2025

Perfect ! It works 😉

@alxlion alxlion merged commit 5cf4759 into ClaperCo:dev Dec 6, 2025
1 check passed
@raulrpearson raulrpearson deleted the allow-generic-s3-endpoints branch December 6, 2025 19:11
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.

2 participants