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

Use configurable Redis in help view #2

Merged
merged 2 commits into from
Jul 8, 2016

Conversation

daf
Copy link
Contributor

@daf daf commented Jun 30, 2016

Having this hardcoded to localhost was messing up my Dockerized setup, so I made it configurable. I'm re-using the redis.sessions.host configuration as that's likely the same instance of Redis that will be used, although technically it should probably be it's own configuration element. Let me know if you'd like this to be updated.

@JamesMakela-NOAA
Copy link
Contributor

@daf: Chris & I talked this over briefly, and we think this is a good idea in principle. I would like to touch on a couple of small things though.

  • It looks like this modification requires a host from the config, but then allows a default for the port number. Is there any reason not to require(or default) the host and the port in the same fashion?
  • If we have code that requires the presence of a config, then it would be nice if the default config file, config-example.ini, have good default configuration settings. The idea is that a developer should be able to just run the thing with a minimum of setup time.

@daf
Copy link
Contributor Author

daf commented Jul 8, 2016

@JamesMakela-NOAA sure, makes sense. Updated to:

  • have a default for host (localhost)
  • use their own settings keys instead of co-opting redis.sessions (redis.help)
  • examples in config-example.ini

@ChrisBarker-NOAA
Copy link
Contributor

thanks Dave!

-CHB

On Fri, Jul 8, 2016 at 11:19 AM, Dave Foster notifications@github.com
wrote:

@JamesMakela-NOAA https://github.com/JamesMakela-NOAA sure, make sense.
Updated to:

  • have a default for host (localhost)
  • use their own settings keys instead of co-opting redis.sessions (
    redis.help)
  • examples in config-example.ini


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA38YBylyZo9Q-Psa9csqExhhzrmzJL8ks5qTpSggaJpZM4JCVZr
.

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov

@JamesMakela-NOAA JamesMakela-NOAA merged commit fe112e8 into NOAA-ORR-ERD:master Jul 8, 2016
@JamesMakela-NOAA
Copy link
Contributor

Yes, that will be satisfactory. Merged it.

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.

3 participants