Skip to content
This repository was archived by the owner on Feb 22, 2019. It is now read-only.

Remove unused nsq common directory. Fix nsqd data_path#15

Open
blakesmith wants to merge 2 commits into
simplereach:masterfrom
blakesmith:nsqd_data_path
Open

Remove unused nsq common directory. Fix nsqd data_path#15
blakesmith wants to merge 2 commits into
simplereach:masterfrom
blakesmith:nsqd_data_path

Conversation

@blakesmith

Copy link
Copy Markdown

Hey there,

In our production environment, we had been setting ['nsq']['nsqd']['data_path'] in our node attributes, and not seeing the data directory being created when provisioning a server with nsqd. This PR fixes that.

I removed the nsq 'common' directory attribute ['nsq']['data_path'], since it's not used anywhere except in the nsqd recipe - it's confusing and not helpful to have one more layer of indirection between the two different directories. The nsqd recipe was updated to create the nsqd data_path instead.

This will be a breaking change for anyone who has overridden ['nsq']['data_path'] instead of ['nsq']['nsqd']['data_path'] - but it seems simpler and more correct to me.

Let me know what you think!

Blake

@elubow

elubow commented Dec 8, 2015

Copy link
Copy Markdown
Contributor

Good catch. I've been meaning to adjust this. Can you please update the metadata.rb and the CHANGELOG.md and the rest of the relevant files so people know?

@blakesmith

Copy link
Copy Markdown
Author

@elubow Updated. I only added a CHANGELOG item, since I've got a few more patches I want to submit and figured you'd want to hold off on version bumping in the metadata.rb until then.

@blakesmith

Copy link
Copy Markdown
Author

Any updates on this?

@mreiferson

Copy link
Copy Markdown
Contributor

Given the fact that it's breaking backwards compatibility (and for a significant option no less), should we bump a major version?

@blakesmith

Copy link
Copy Markdown
Author

@mreiferson Major version bump sounds reasonable to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants