I have a discourse instances where, months after initially creating the instance, we enabled “S3” (in our case, digital ocean spaces) for a while, only via admin settings, and never with any
DISCOURSE_ config options in the app.yml file. We never migrated everything to S3, but the bulk of our images were written while we had it enabled. Now we want to migrate everything off “S3” for a variety of reasons not important here.
(I know this sounds like a support post so far. But it’s not…)
In a recently-merged PR, I added a rake task
uploads:batch_migrate_from_s3 as a trivial wrapper around
uploads:migrate_from_s3 to be able to migrate batches as small as only one post worth of images, and ran it. At this point I discovered that
upload:migrate_from_s3 which my task calls assumes that discourse.conf has S3 settings in it that aren’t available in the UI:
** Invoke uploads:batch_migrate_from_s3 (first_time) ** Invoke environment (first_time) ** Execute environment ** Execute uploads:batch_migrate_from_s3 Migrating uploads from S3 to local storage for 'default'... rake aborted! NoMethodError: undefined method `downcase' for nil:NilClass /var/www/discourse/app/models/global_setting.rb:107:in `s3_bucket_name' /var/www/discourse/app/models/site_setting.rb:157:in `absolute_base_url' /var/www/discourse/lib/tasks/uploads.rake:138:in `migrate_from_s3' /var/www/discourse/lib/tasks/uploads.rake:118:in `block in migrate_all_from_s3'
What is the best solution?
- It’s expected that S3 setup is always done through
DISCOURSE_environment variables in the app.yml file and written into
config/discourse.confwhile building the container, and I should do that and rebuild my app now. (I note that the environment variable input to rake doesn’t seem to set
GlobalSettingshere either; I’ve tried that too. Presumably, the fact that anything worked with only
SiteSettingsconfigured is an oversight and unintended.) I tested this by putting
config/discourse.confin my existing container and it resolved the error for me.
- Modify the migrate task further so that if
SiteSetting.s3_upload_bucketis set but
SiteSetting.s3_upload_bucket— I’m guessing this wouldn’t be a hard PR if it’s considered right, but I haven’t looked hard yet. Edit: I tried modifying
uploads:migrate_from_s3and that didn’t work because of lack of an accessor.
nil— is this even intended to be a right configuration?
- Add the ability to set
s3_bucketin site settings so that it’s exposed in the UI.
The first is just site configuration, and I’ll probably just do it. If the second is considered a good alternative I’m happy to take a crack at PRing it. However, I’m new enough here that I’m not confident in making changes as core as the latter two; the expectations for how the config objects work are a bit opaque to me.
To be clear: I’ve worked trivially around my problem; it just surprised me that it was necessary.