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

Set memcached server override in ENV["MEMCACHED_SERVER"] rather than sed #149

Merged
merged 1 commit into from
Jun 21, 2017

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented May 30, 2017

We shouldn't edit settings.yml as it's the default shipping with the product.

@bdunne bdunne requested review from Fryguy and carbonin May 30, 2017 18:41
@carbonin
Copy link
Member

Would this be better as a docker-asset? Or do you want to keep this openshift-ish stuff away from the pure docker stuff?

@bdunne
Copy link
Member Author

bdunne commented May 30, 2017

I think ERB rendering in YAML is disabled in production mode, and I don't want to ship a hard-coded value since the environment variable could change.

@Fryguy
Copy link
Member

Fryguy commented Jun 3, 2017

I think it might be better to store it in config/settings/production.local.yml . I like to leave settings.local.yml for users.

@carbonin
Copy link
Member

carbonin commented Jun 9, 2017

@bdunne Was there a reason to keep this file in settings.local.yml rather than take @Fryguy 's suggestion of production.local.yml?

@bdunne bdunne force-pushed the use_settings_local_yml branch from b635677 to a1d6591 Compare June 20, 2017 16:08
@miq-bot
Copy link
Member

miq-bot commented Jun 20, 2017

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

@bdunne
Copy link
Member Author

bdunne commented Jun 20, 2017

With the changes merged in ManageIQ/manageiq#15326 we can use an ENV variable instead.

@Fryguy
Copy link
Member

Fryguy commented Jun 20, 2017

@bdunne Can you change the PR title to reflect the new direction?

@bdunne bdunne changed the title Set memcached server override in settings.local.yml rather than sed Set memcached server override in ENV["MEMCACHED_SERVER"] rather than sed Jun 20, 2017
@carbonin carbonin added this to the Sprint 64 Ending Jul 3, 2017 milestone Jun 21, 2017
@carbonin carbonin merged commit 1d20eec into ManageIQ:master Jun 21, 2017
@bdunne bdunne deleted the use_settings_local_yml branch June 21, 2017 13:24
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