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

Add accumulator and resource for writing storage-aggregation.conf #192

Closed
wants to merge 1 commit into from
Closed

Add accumulator and resource for writing storage-aggregation.conf #192

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 9, 2014

First of all, thank you for the awesome rewrite of the cookbook! The accumulator pattern to generate the INI files is fantastic.

This essentially copies the functionality of the resources/providers that write out storage.conf, in order to provide the same pattern to write out storage-aggregation.conf. I found myself in need of this tonight so that I could configure storage-aggregation.conf for Statsd.

Let me know if you'd like any additional testing for this, or if I'm completely off! I would also be happy to write accumulator resources for the other configuration files that aren't covered yet (in which case some additional DRYing of the accumulator code might be in order as well).

@webframp
Copy link
Contributor

Nice, will test this out. Thanks

@webframp
Copy link
Contributor

webframp commented Oct 8, 2014

Given the discussion ongoing in #197 it may be best to hold off on merging this. Functionally things would be similar, but which stage things happen at could change

@iksaif
Copy link

iksaif commented Dec 29, 2015

Any update on this ? It would be very nice to have and this has been on hold for almost a year.

@bradenwright
Copy link

So over 2 years now, any word on this?

@webframp
Copy link
Contributor

I don't mind this addition as an intermediate improvement if it helps someone use this cookbook, but I have been waiting on input from others working on #228 and #230 since that would definitely impact this PR.

@tas50
Copy link
Contributor

tas50 commented Oct 30, 2016

I think at this point we should just merge this in and then refactor things into proper custom resource based accumulators later

@Esya
Copy link

Esya commented Dec 8, 2016

I think it's about time to merge this..!

@lamont-granquist
Copy link

The existing accumulator pattern is so bad that I'm very 👎 on merging this kind of functionality until that all gets rewritten. Adding this just makes that harder and makes the existing architectural damage even worse.

I think it will need to be rewritten as custom resources that implement accumulators sooner rather than later for Chef 13 as well.

@webframp
Copy link
Contributor

I think I'm in agreement with @lamont-granquist on this one. Top priority should be rewriting/replacing accumulator pattern based on his earlier suggestions for a more manageable approach.

It was a naive attempt at custom resources for managing graphite. If you're interested in more back story for any reason, perhaps @fnichol still remembers. I no longer work with graphite day to day and haven't for several years.

@damacus
Copy link
Member

damacus commented Apr 27, 2017

I'm going to look at pulling this logic into the custom resource rewrite (#276)

Closing this down at it'll need a significant change to work.

Thanks very much for your help!

@damacus damacus closed this Apr 27, 2017
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.

8 participants