Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding memcache server setting by environment variable #15326

Merged
merged 1 commit into from
Jun 20, 2017

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Jun 6, 2017

In a production container environment, we want to set an environment variable to give the app the memcached server address and port rather than writing config/settings/production.yml since that file is checked in and could conflict.

@miq-bot
Copy link
Member

miq-bot commented Jun 6, 2017

Checked commit bdunne@f2fd7ff with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks fine. 👍

@@ -2,6 +2,10 @@
require 'linux_admin'

module MiqMemcached
def self.server_address
ENV["MEMCACHED_SERVER"] || ::Settings.session.memcache_server
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is just for containers should we use the ENV["CONTAINER"] here and just switch this directly to using ENV["MEMCACHED_SERVICE_NAME"] which is the variable this is already exposed as in the container?

Or even just s/SERVER/SERVICE_NAME/ and assume it won't be there in any other environment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, MEMCACHED_SERVICE_NAME is only "memcached". It doesn't include the port. I was trying to make this a more generic solution, like how Rails uses DATABASE_URL.

@jrafanie
Copy link
Member

LGTM

@jrafanie jrafanie merged commit 091c64f into ManageIQ:master Jun 20, 2017
@jrafanie jrafanie added this to the Sprint 64 Ending Jul 3, 2017 milestone Jun 20, 2017
@bdunne bdunne deleted the memcache_server_env_variable branch June 20, 2017 15:55
carbonin added a commit to carbonin/manageiq that referenced this pull request Apr 30, 2018
MiqMemcached.server_address was introduced in ManageIQ#15326 which is not
in fine
d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
MiqMemcached.server_address was introduced in ManageIQ#15326 which is not
in fine
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants