search

nkrode / RedisLive

Visualize your redis instances, analyze query patterns and spikes.
http://www.nkrode.com/article/real-time-dashboard-for-redis
MIT License
3.07k stars 644 forks source link

Add support for Environment and YAML settings. Closes #29 #37

Closed josegonzalez closed 12 years ago

josegonzalez commented 12 years ago

Note that this adds pyyaml as a dependency. The settings importing can take that into account with a hacky try/except ImportError around yaml, but I'd rather not do that.

kumarnitin commented 12 years ago

I agree YAML is simpler for some people, however at this point many people are using RedisLive using the existing settings format. At this point I don't think it's worth the trouble to break backward compatibility with existing settings files.

Thanks!

josegonzalez commented 12 years ago

Please reopen this. There were no BC breakages with this PR, except to add support for another configuration file type.

kumarnitin commented 12 years ago

You now have two config files, src/redis-live.json and src/redis-live.yaml. I don't see a need to have two files.

The breaking change will be for people who are already using the existing format for the config files, now they will need to redo their config files or support two files.

I like YAML, I should have gone with YAML from the start but at this point switching to YAML doesn't really have any benefit, instead it forces existing users to rework their configs for no real gain.

josegonzalez commented 12 years ago

No. You can use either, you do not need to include both. I can add documentation if need be for yaml support. No one needs to rewrite anything to support yaml on their end. This change is non-intrusive.

kumarnitin commented 12 years ago

I get it, but your commit introduces two configuration files. The master branch needs to remain clean. If we support the YAML file then we'll need to remove the .conf -> .json file, which will end up breaking things for existing users.

This change can be included in the next major release, when things are expected to break. For example support for Redis 2.6 will be a breaking change, so we can introduce YAML then. Also adding a couple of new widgets with the next major release, so that it's worth an upgrade.

josegonzalez commented 12 years ago

Why exactly do you need to remove a feature to add this? On Oct 29, 2012 3:47 PM, "Nitin Kumar" notifications@github.com wrote:

I get it, but your commit introduces two configuration files. The master branch needs to remain clean. If we support the YAML file then we'll need to remove the .conf -> .json file, which will end up breaking things for existing users.

This change can be included in the next major release, when things are expected to break. For example support for Redis 2.6 will be a breaking change, so we can introduce YAML then. Also adding a couple of new widgets with the next major release, so that it's worth an upgrade.

— Reply to this email directly or view it on GitHubhttps://github.com/kumarnitin/RedisLive/pull/37#issuecomment-9882692.