[#140196, #140197] extract secrets from settings.yml #1453
Conversation
meara
changed the title
| # - account_manager | ||
|
|
||
| # This may be overridden in settings.local.yml if your fork is using S3, so | ||
| # This may be overridden in secrets.yml if your fork is using S3, so |
There was a problem hiding this comment.
This part isn't quite true as it stands. I think it might be nice to think about how this can work.
What might work is to do a Settings.paperclip.merge(Rails.application.secrets.paperclip) and then we can get the best of both worlds. Thoughts on that?
There was a problem hiding this comment.
Hmm. Where would we actually do the merge?
There was a problem hiding this comment.
Looks like we use the paperclip settings in app/models/concerns/downloadable_file.rb. So probably in there.
|
Looks good. Does it make sense to add a "Deploy Note" that changes will need to be made to |
|
Good call on adding a "Deploy Note" @giladshanan. |
jhanggi
left a comment
jhanggi
left a comment
There was a problem hiding this comment.
One tiny thing we might be able to clean up, but otherwise looks good.
|
|
||
| included do | ||
| has_attached_file :file, Settings.paperclip.to_hash.merge(validate_media_type: false) | ||
| has_attached_file :file, PaperclipSettings.config.to_hash.merge(validate_media_type: false) |
There was a problem hiding this comment.
The to_hash here might now be unnecessary.
meara
force-pushed
the
extract-secrets-from-settings
branch
from
3f41766 to
afd0f3d
Compare
Release Notes
Secret information such as API keys should exist in
secrets.ymlrather than insettings.ymlor by being overridden insettings.local.yml.Deploy Note
Will need to update
settings.ymlandsecrets.ymlto match the updated templates on staging and production servers in conjunction with the release.