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 more customizations in config.js #3

Closed
wants to merge 13 commits into from
Closed

Conversation

kronn
Copy link

@kronn kronn commented Aug 22, 2014

Background and Motivation

Grafana can be configure in many different ways, but the basic setup is always the same.

In order to use your module,
I want to pass in my own config-template
But still use the class to define the essential bits from the manifest.

Implementation

I added *_scheme-parameters for graphite, elasticsearch, and influxdb. I have Graphite and ElasticSearch running on a https-host, so I need that ability.

I also do not use InfluxDB, so I made the config-template overridable. This allows me to use our own template. In the template, I can still use the parameters defined in the class.

Side effects

Having the template overridable is essentially giving people enough rope to hang themselves. Then again, the target audience is admins and devops-teams, so we're all capable of handling this.

This is a preparation to include that in the config-template.
this way, users can choose their own settings more easily.
@bfraser
Copy link
Owner

bfraser commented Aug 23, 2014

Thanks for the pull request.

URL

I think the easiest and most effective way to give the user flexibility and control over the Graphite, Elasticsearch and InfluxDB URLs is to simply provide a url parameter. This should address the protocol/scheme which was the goal of your request, and provides coverage for users who may be using basic authentication etc.

Template

I understand your desire to have more control over the template. Perhaps, rather than parameterizing the template file to use, the existing template can be made better. My thinking was to introduce boolean parameters that control whether graphite, elasticsearch, and / or influxdb datasources are to be used, and have logic in the template that only includes them and makes use of their associated parameters if true.

Would these changes provide sufficient coverage for your use case, or do you still see a need for *_scheme parameters and parameterizing the template file?

Thanks!

@kronn
Copy link
Author

kronn commented Aug 25, 2014

The change you propose would be sufficient as well. I will try to come up with something later today and update this PR accordingly.

@kronn
Copy link
Author

kronn commented Aug 25, 2014

There, I fixed it ;-)

@kronn
Copy link
Author

kronn commented Aug 26, 2014

This should make Travis happy.

@kronn
Copy link
Author

kronn commented Aug 26, 2014

Ah, finally green. By now I must have touched almost all files of the module :-)

What do you think of the changes, @bfraser ?

@bfraser
Copy link
Owner

bfraser commented Aug 26, 2014

I'll try to have a look at this later today. Thanks for the diligence :-)

@bfraser
Copy link
Owner

bfraser commented Aug 29, 2014

Sorry, I haven't had a chance to get to this yet. Probably not until the weekend.

@kronn
Copy link
Author

kronn commented Aug 29, 2014

No problem. I'm using my own fork in the meantime. I just meant to be a good open-source citizen, not to stress you out.

If you spot more problems, just say it.

For now, I'm just happy that it works the way I want :-)

@analogue
Copy link
Contributor

analogue commented Sep 3, 2014

FYI, I little bit more flexible implementation for datasources - #4

@kronn
Copy link
Author

kronn commented Sep 4, 2014

That looks great, @analogue. I will update this PR to only cover the admin_password (because I use that). You solved the datasource-problem way more flexible than I did.

@bfraser
Copy link
Owner

bfraser commented Apr 23, 2015

Closing this out as the module now works with Grafana 2.x and there no longer is a config.js to manage.

@bfraser bfraser closed this Apr 23, 2015
@kronn
Copy link
Author

kronn commented Apr 24, 2015

👍

rplessl added a commit to rplessl/puppet-grafana that referenced this pull request Jun 7, 2015
cegeka-jenkins pushed a commit to cegeka/puppet-grafana that referenced this pull request Oct 23, 2017
bastelfreak pushed a commit to bastelfreak/puppet-grafana that referenced this pull request Jul 28, 2019
* initial commit to implement grafana_folder resource type

* add required parameters for grafana objects

* remove references to dashboard objects (dashboard used as template)

* update tests

* remove whitespace for rubocop

* rubocop fixes

* fix global var assignment clause logic

* update README.md/created acceptance test for grafana_folder resource
cegeka-jenkins pushed a commit to cegeka/puppet-grafana that referenced this pull request Feb 1, 2024
* initial commit to implement grafana_folder resource type

* add required parameters for grafana objects

* remove references to dashboard objects (dashboard used as template)

* update tests

* remove whitespace for rubocop

* rubocop fixes

* fix global var assignment clause logic

* update README.md/created acceptance test for grafana_folder resource
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