Clean merge for PR#59 'Expanded includes'

[falkenbt]

I tried to merge #59 . Can you please verify if everything is working as expected? I successfully tested it on a small installation, but since it's a rather big merge and I would like to make sure there's no regression.

[cornet]

Merged this in locally and nothing broke (admitted a very small install). Just testing out the new features

[cornet]

I've not looked into fixing this yet but found a problem earlier when creating dashboards from templates. While the dashboard shows if you click on a graph it tries to .graph file from the dashboard directory.

On first sight the offending method is graph_by_name in lib/gdash/dashboard.rb as it knows nothing of the :include_graphs: property so never searches in the included directories for the graph files.

I'll see if I can come up with a fix for this.

This potentially poses another question as to whether the names of .graph files need to be unique or not ?

[falkenbt]

I can't assure that I merged everything correctly, maybe I missed something and we should look into the original pull request. Am 07.02.2013 00:06 schrieb "Nathan" notifications@github.com:

I've not looked into fixing this yet but found a problem earlier when creating dashboards from templates. While the dashboard shows if you click on a graph it tries to .graph file from the dashboard directory.

On first sight the offending method is graph_by_name in lib/gdash/dashboard.rb as it knows nothing of the :include_graphs: property so never searches in the included directories for the graph files.

I'll see if I can come up with a fix for this.

This potentially poses another question as to whether the names of .graph files need to be unique or not ?

— Reply to this email directly or view it on GitHubhttps://github.com/ripienaar/gdash/pull/66#issuecomment-13210994.

[cornet]

First pass at getting the details graph pages working:

https://github.com/cornet/gdash/compare/ripienaar:PR59...cornet:PR59-fixes

[falkenbt]

Thank you for the fix @cornet! Works on my machine.

Do we get a +1 for merging this into master, @powdahound @keymon @bwhaley? Want to do some more testing?

[bwhaley]

+1, I've tested this code and it worked for me. Nice work.

The README.md could use a little clean up I think though. In the additional properties section, it says

You can specify additional properties in the dash.yaml:

    :graph_properties:
        :environment: dev
        :server: [ a_server_name ]
        :javaid: 1234

that can be accessed from the .graph like:

    server = @properties[:server]
    environment = @properties[:environment]

    field :iowait, 
        :alias => "IO Wait #{server}",
        :data  => "servers.#{environment}.#{server}.cpu*.cpu-{system,wait}.value"

But in fact, if :server: is an array, you'll need something like

server_definition = (servers.is_a? Array) ? "{#{servers.join(',')}}" : servers

to use it in the graph (per the original PR from @keymon).

Also, in the "Include graphs" section, it wasn't immediately clear to me that these needed to be directories full of additional .graph files. At first I thought the graph contents were in those files.

Finally, :include: needs to be updated to :include_graphs: in the README.md.

[keymon]

+1 Tested in our configuration, it works OK. I am with @bwhaley about the README updates.

In my branch I got other features, I guess the best option is reimplement them based on this code.

[keymon]

I fixed the README and commented about the :include_properties: feature in other PR: #68

[falkenbt]

:+1: Very nice.

I would go ahead and merge this into master. (Force forward before we diverge again). Any objections?

[powdahound]

:+1: