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

Moving computation of download URL from attributes to recipe #240

Closed
wants to merge 4 commits into from
Closed

Moving computation of download URL from attributes to recipe #240

wants to merge 4 commits into from

Conversation

jakekdodd
Copy link

Computation of the download filename and download URL in the defaults.rb attributes file was causing confusing behavior upon node convergence. If the user overrode the node.elasticsearch[:version] property, the download directory and symlinks would be created with the correctly overridden version number. However, the download URL would be computed with the default version number, and thus the incorrect, default version of elasticsearch would be installed.

Since there's no need to compute the download filename and URL until node convergence, this pull request moves that computation to the default.rb recipe. However, the node.elasticsearch[:filename] and node.elasticsearch[:download_url] attributes are still recognized and given preference upon node convergence, so that users still have the capability to override these attributes from nil and thus configure downloads from a custom repository.

Computing attributes like this, in the attributes file, causes really confusing behavior at runtime. Setting ``elasticsearch[:download_url]`` to nil, and computing at runtime instead. User can override the attribute in a wrapper cookbook and the recipe will actually respect that override.
@bai
Copy link

bai commented Oct 6, 2014

Could you please squash these into a single logical commit? Otherwise LGTM.

@jakekdodd
Copy link
Author

Will do--closing this pull request and submitting a fresh one with a single commit.

@jakekdodd jakekdodd closed this Oct 7, 2014
karmi pushed a commit that referenced this pull request Oct 13, 2014
This prevents a number of errors and surprises when setting the Elasticsearch version.

If the user overrode the node.elasticsearch[:version] property, the download directory and symlinks would be
created with the correctly overridden version number. However, the download URL would be computed with
the default version number, and thus the incorrect, default version of elasticsearch would be installed.

Since there's no need to compute the download filename and URL until node convergence, this patch moves that computation to the default.rb recipe. However, the node.elasticsearch[:filename] and node.elasticsearch[:download_url] attributes, if present, are respected and given preference upon node convergence, so that users still have the capability to override these attributes and thus configure downloads from a custom repository.

Related: #100, #180, #178, #240, #179, #245, #227, #245

Closes #242
@karmi
Copy link
Contributor

karmi commented Oct 13, 2014

Closed in #242 (eb65bbd)

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.

None yet

3 participants